Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

TypeScript error on fresh install #6244

Closed
1 task done
MichaelDeBoey opened this issue Apr 29, 2023 · 11 comments
Closed
1 task done

TypeScript error on fresh install #6244

MichaelDeBoey opened this issue Apr 29, 2023 · 11 comments

Comments

@MichaelDeBoey
Copy link
Member

MichaelDeBoey commented Apr 29, 2023

What version of Remix are you using?

Latest

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

Install a new Remix project & run npm run typecheck

npx create-remix@latest
? Where would you like to create your app? ./my-remix-app
? What type of app do you want to create? Just the basics
? Where do you want to deploy? Choose Remix App Server if you're unsure; it's easy to change deployment targets. Remix App Server
? TypeScript or JavaScript? TypeScript
? Do you want me to run `npm install`? Yes
cd ./my-remix-app 
npm run typecheck

Expected Behavior

No type-errors would appear

Actual Behavior

node_modules/@remix-run/node/dist/upload/fileUploadHandler.d.ts:47:22 - error TS2420: Class 'NodeOnDiskFile' incorrectly implements interface 'File'.
  Property 'prototype' is missing in type 'NodeOnDiskFile' but required in type 'File'.

47 export declare class NodeOnDiskFile implements File {
                        ~~~~~~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:2572:5
    2572     prototype: Blob;
             ~~~~~~~~~
    'prototype' is declared here.


Found 1 error in node_modules/@remix-run/node/dist/upload/fileUploadHandler.d.ts:47

This is the same error we got in #6108 & that @dpobel also reported in #6119 (comment)

The problem seemed to be that NodeOnDiskFile doesn't have a prototype property, but after implementing that (in #6108), we still get

Type 'NodeOnDiskFile' provides no match for the signature 'new (blobParts?: BlobPart[] | undefined, options?: BlobPropertyBag | undefined): Blob'.

Since @jacob-ebey is the creator of the class, maybe he can point me/us into the right direction here
I tried a lot, but I can't seem to find a solution to make this work tbh 🤔

@MichaelDeBoey MichaelDeBoey added the bug Something isn't working label Apr 29, 2023
@MichaelDeBoey
Copy link
Member Author

After doing some investigation, I came to the conclusion it couldn't be a problem with typescript itself (even though the error is saying so) as that code never changes 🤷‍♂️

It's always the same code:

typescript/lib/lib.dom.d.ts
interface Blob {
  readonly size: number;
  readonly type: string;
  arrayBuffer(): Promise<ArrayBuffer>;
  slice(start?: number, end?: number, contentType?: string): Blob;
  stream(): ReadableStream<Uint8Array>;
  text(): Promise<string>;
}

declare var Blob: {
  prototype: Blob;
  new (blobParts?: BlobPart[], options?: BlobPropertyBag): Blob;
};

type BlobPart = BufferSource | Blob | string;

interface BlobPropertyBag {
    endings?: EndingType;
    type?: string;
}

type BufferSource = ArrayBufferView | ArrayBuffer;

type EndingType = "native" | "transparent";

interface File extends Blob {
  readonly lastModified: number;
  readonly name: string;
  readonly webkitRelativePath: string;
}

declare var File: {
  prototype: File;
  new (fileBits: BlobPart[], fileName: string, options?: FilePropertyBag): File;
};

interface FilePropertyBag extends BlobPropertyBag {
    lastModified?: number;
}

Using package.json's overrides to specify the installed @types/node version, it seems we only have this problem with having v18 installed.

It seems like @vansergen introduced File in @types/node v18 (see DefinitelyTyped/DefinitelyTyped#64319), which I assume is causing the problem now.
When looking at the implemented types in @types/node, they seem to be compatible to me (but maybe I'm missing some things?), so I don't really know what the actual problem could be here tbh 🤔

@types/node/buffer.d.ts
export interface FileOptions {
  /**
   * One of either `'transparent'` or `'native'`. When set to `'native'`, line endings in string source parts will be
   * converted to the platform native line-ending as specified by `require('node:os').EOL`.
   */
  endings?: "native" | "transparent";
  /** The File content-type. */
  type?: string;
  /** The last modified date of the file. `Default`: Date.now(). */
  lastModified?: number;
}
/**
 * A [`File`](https://developer.mozilla.org/en-US/docs/Web/API/File) provides information about files.
 * @experimental
 * @since v18.13.0
 */
export class File extends Blob {
  constructor(
    sources: Array<BinaryLike | Blob>,
    fileName: string,
    options?: FileOptions
  );
  /**
   * The name of the `File`.
   * @since v18.13.0
   */
  readonly name: string;
  /**
   * The last modified date of the `File`.
   * @since v18.13.0
   */
  readonly lastModified: number;
}
@types/node/crypto.d.ts
type BinaryLike = string | NodeJS.ArrayBufferView;

@MichaelDeBoey MichaelDeBoey changed the title Class 'NodeOnDiskFile' incorrectly implements interface 'File'. TypeScript error on fresh install Apr 29, 2023
@jacob-ebey
Copy link
Member

The issue is it doesn't extend the DOM types directly so it's missing the prototype. @pcattori do you know of a way to trick typescript into adding the prototype to a non-related class?

@MichaelDeBoey
Copy link
Member Author

Doing some more investigation, I landed on DefinitelyTyped/DefinitelyTyped#65049, which lead me to @pcattori's microsoft/TypeScript#52166

According to that issue, it doesn't seem to be a problem with @node/types, but it seems to be a regression in TS that's failing to recognize the fact that we're doing an implements of the interface instead of the var.

As @pcattori pointed out: things are working as expected with TS 4.8.4 in our codebase (this is also confirmed after I did that in #6108)
When setting TS to v4.8.4 in a template, we still get the same error though 🤔

@MichaelDeBoey
Copy link
Member Author

Did some further investigation and landed on #4371.

Here @akomm points out (see #4371 (comment) & #4371 (comment)) that it would be a collision between typescript/lib/lib.dom.d.ts & @types/node/buffer.d.ts, as you can see in the TS Playground example he posted.

Basically having types for any built-in Node module is causing this error.

@jacob-ebey
Copy link
Member

Can we just // @ts-ignore in the remix package on the implements line?

@akomm
Copy link

akomm commented May 2, 2023

It would be probably better than blocked build or having to entirely disable typecheck on production build. But the types are actually wrong as the prototype isn't properly picked, so writing code that operates on the "wrong" types could lead to unexpected bugs. Just saying. It should be removed once the issue is resolved on the TS side.

@MichaelDeBoey
Copy link
Member Author

MichaelDeBoey commented May 6, 2023

@jacob-ebey Adding @ts-ignore on the implements line still throws errors on other lines.

The proper way would be to actually fix NodeOnDiskFile class

@lpsinger
Copy link
Contributor

lpsinger commented Jun 16, 2023

According to microsoft/TypeScript#52166:

This changed between TS 4.8.4 and TS 4.9.3. Typechecking passed in 4.8.4, fails in 4.9.3+

Is it an option to roll the TS back to 4.8.4 in the Remix dependencies?

@MichaelDeBoey
Copy link
Member Author

@lpsinger One of the reasons we updated to V5 was to support extends array in tsconfig.json
Reverting to TS v4 would revert that support as well 😕

@jacob-ebey Is there any other solution you can think of besides adding @ts-ignore?
Can't really think of anything else tbh 🤔

@brophdawg11 brophdawg11 removed their assignment Jun 20, 2023
@pcattori
Copy link
Contributor

pcattori commented Jul 3, 2023

Duplicate of #4371

@pcattori pcattori marked this as a duplicate of #4371 Jul 3, 2023
@pcattori pcattori closed this as completed Jul 3, 2023
@MichaelDeBoey
Copy link
Member Author

This is now fixed by #6108 and will be released in v2

# for free to join this conversation on GitHub. Already have an account? # to comment