Typescript compilation fails #4371
Comments
ananni13
changed the title
|
I successfully reproduced the issue with NodeFileOnDisk 👍 However, I'm not sure we will be able to fix errors from Cloudflare pages & workers, as they seem to emanate from the |
|
You can add Though I'm not sure if it's a bad practice to ignore libs. |
|
@depsimon yes this is terrible as it affects not just libs, but simply all |
|
I'm getting this with a Running TS 4.9.4 and Remix 1.9.0 |
|
A quick workaround I did was to just add define the property as null. In The class ends up looking something like: |
This comment was marked as duplicate.
This comment was marked as duplicate.
|
Looking into this, the offending line is: export declare class NodeOnDiskFile implements File {which references the /** A file-like object of immutable, raw data. Blobs represent data that isn't necessarily in a JavaScript-native format. The File interface is based on Blob, inheriting blob functionality and expanding it to support files on the user's system. */
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;
};
// ...
/** Provides information about files and allows JavaScript in a web page to access their content. */
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;
};What's weird is that the interfaces being referenced don't have a Workaround is to use |
|
Downgrading Workaround is to force TS 4.8: |
|
Here's a TS playground that shows that our For some reason, this same definition within |
|
@pcattori this is correct ts behavior. // MyType.ts
export type MyType = typeof MyType
export const MyType = {
foo: "bar"
}
// test.ts
import {MyType} from "./MyType"
function takesMyType(myType: MyType) {
return MyType.foo
}TS will distinguish between the type and value based on the whether it is used in type or value context. |
|
@akomm right, TS should distinguish between type and value based on context. But in this case |
declare var File: {
prototype: File;
new(fileBits: BlobPart[], fileName: string, options?: FilePropertyBag): File;
};
class MyConstructor {} // syntax sugar for the constructor function
console.log(MyConstructor)
console.log(MyConstructor.prototype)
console.log(MyConstructor.name)
interface Something extends MyConstructor {
}Regarding the problem itself. The problem is that It might be already enough to change from |
|
Changing to extends in my cases fixes the error. Because you say I think what TS changed is that if checks whether there is a constructor in the same namespace with the same name, to figure out its supposed to be extended to highlight the potentially previously done mistakes. |
|
@akomm my main confusion is why TS is happy with Not sure why TS points to Here's another playground that shows that TS normally uses the |
|
I see the confusion. There might be a secondary problem. First, as explained it should be TS Playground does not have all those ambient libraries you have in your code. There is just a preset of "lib" which you can't set in the Weird things can happen, when wrong global types are picked or declarations become merged. At least one of those things I could find already: // lim.dom.d.ts
interface File extends Blob {
readonly lastModified: number;
readonly name: string;
readonly webkitRelativePath: string;
}The By the way: did you check TS language service trace or just what the IDE makes up of it? There are sometimes multiple pointers to different contexts provided. For example the declaration of Another one with // lib.dom.d.ts
declare var Blob: {
prototype: Blob;
new(blobParts?: BlobPart[], options?: BlobPropertyBag): Blob;
};Also |
|
Its basically a collision with |
|
A small change in the TS Playground example makes the error appear. I assume the Playground loads node library once it sees any usage of its core modules. You can change "buffer" with "path" or any other core module of node, the effect is the same. |
|
@akomm I think the issue is that we do want to use Talked to @jacob-ebey about this a couple days ago and we didn't know of a good way to disambiguate the different |
Lower tsc version to workaround `remix-run/remix#4371
|
Confirming that Typescript 4.9+ is an issue Steps to ReproduceCreated a fresh remix app. I chose the remix express template
After downgrading to typescript |
|
If you don't want to downgrade Typescript but want to stop the error and get a nudge when this is fixed then put You can leave your self a reminder of why too if you like: Just be careful if you have multiple type checks on the next line as it will stop errors for any of the checks. |
|
What about defining export class NodeOnDiskFile implements File {
prototype = {};
name: string;
...
} |
|
@lpsinger unfortunately, 
To recap for any onlookers, Looks like TS has included this regression as part of their 5.1.0 milestone, though not sure about the timing of that since 5.1.0 is already out? Maybe 5.1 milestone includes things to fix before 5.2? 🤷 |
|
This is now fixed by #6108 and will be released in v2 |
|
@MichaelDeBoey I think I'm still getting these issues on remix-run/cloudflare |
|
@dkjym Please open a new issue and give more details 🙏🏼 |
What version of Remix are you using?
1.7.2, Node v16.18.0, npm 8.19.2
Steps to Reproduce
Run
npx create-remix@latest, pick Just the basics, any template, Typescript.Run
npx tsc --buildinside the app folder.Expected Behavior
Typescript compiles without errors.
Actual Behavior
Output with Remix App Server, Express, Architect, Fly, Netlify and Vercel templates:
Expand
Output with Cloudflare Pages and Workers templates:
Expand
The text was updated successfully, but these errors were encountered: