-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates vfs to be more future safe when running on node + 2021 dts files for playground #2802
Conversation
orta
commented
Apr 25, 2023
- On node a readdir is possible, so we can ignore the known libs completely, making it future proof
- For the browser (aka the playground), the vfs need to know about a bunch of new lib files
…include new .d.ts files on the playground
Truthfully, I don't know the interaction between this file listing and what the playground will use, though; I assume they are related? But, monaco has its own list. So hopefully this won't break? |
The CIs are red because I declared an update to the package json version, I'll yarn install to hit the lockfile. Good point, the playground uses its own ts.Program instance for thing like js/dts emit / ATA etc which is created using the tsvfs dts files, which the program is then used for a bunch of introspection stuff. |
Turns out that I'll work on a snippet that can produce the same-ish list but in old versions. I'll need it for monaco anyway. |
It turns out that there's no way that works across versions to get a full listing of the lib files in the package. The only way I think we can get the same result is to use the new API if available, or as a fallback, encode a huge list of all lib files that have ever existed and verify that they do actually exist before using them. That full list is: [
"lib.d.ts",
"lib.decorators.d.ts",
"lib.decorators.legacy.d.ts",
"lib.dom.d.ts",
"lib.dom.iterable.d.ts",
"lib.es2015.collection.d.ts",
"lib.es2015.core.d.ts",
"lib.es2015.d.ts",
"lib.es2015.generator.d.ts",
"lib.es2015.iterable.d.ts",
"lib.es2015.promise.d.ts",
"lib.es2015.proxy.d.ts",
"lib.es2015.reflect.d.ts",
"lib.es2015.symbol.d.ts",
"lib.es2015.symbol.wellknown.d.ts",
"lib.es2016.array.include.d.ts",
"lib.es2016.d.ts",
"lib.es2016.full.d.ts",
"lib.es2017.d.ts",
"lib.es2017.full.d.ts",
"lib.es2017.intl.d.ts",
"lib.es2017.object.d.ts",
"lib.es2017.sharedmemory.d.ts",
"lib.es2017.string.d.ts",
"lib.es2017.typedarrays.d.ts",
"lib.es2018.asyncgenerator.d.ts",
"lib.es2018.asynciterable.d.ts",
"lib.es2018.d.ts",
"lib.es2018.full.d.ts",
"lib.es2018.intl.d.ts",
"lib.es2018.promise.d.ts",
"lib.es2018.regexp.d.ts",
"lib.es2019.array.d.ts",
"lib.es2019.d.ts",
"lib.es2019.full.d.ts",
"lib.es2019.intl.d.ts",
"lib.es2019.object.d.ts",
"lib.es2019.string.d.ts",
"lib.es2019.symbol.d.ts",
"lib.es2020.bigint.d.ts",
"lib.es2020.d.ts",
"lib.es2020.date.d.ts",
"lib.es2020.full.d.ts",
"lib.es2020.intl.d.ts",
"lib.es2020.number.d.ts",
"lib.es2020.promise.d.ts",
"lib.es2020.sharedmemory.d.ts",
"lib.es2020.string.d.ts",
"lib.es2020.symbol.wellknown.d.ts",
"lib.es2021.d.ts",
"lib.es2021.full.d.ts",
"lib.es2021.intl.d.ts",
"lib.es2021.promise.d.ts",
"lib.es2021.string.d.ts",
"lib.es2021.weakref.d.ts",
"lib.es2022.array.d.ts",
"lib.es2022.d.ts",
"lib.es2022.error.d.ts",
"lib.es2022.full.d.ts",
"lib.es2022.intl.d.ts",
"lib.es2022.object.d.ts",
"lib.es2022.regexp.d.ts",
"lib.es2022.sharedmemory.d.ts",
"lib.es2022.string.d.ts",
"lib.es2023.array.d.ts",
"lib.es2023.d.ts",
"lib.es2023.full.d.ts",
"lib.es5.d.ts",
"lib.es6.d.ts",
"lib.esnext.array.d.ts",
"lib.esnext.asynciterable.d.ts",
"lib.esnext.bigint.d.ts",
"lib.esnext.d.ts",
"lib.esnext.full.d.ts",
"lib.esnext.intl.d.ts",
"lib.esnext.promise.d.ts",
"lib.esnext.string.d.ts",
"lib.esnext.symbol.d.ts",
"lib.esnext.weakref.d.ts",
"lib.scripthost.d.ts",
"lib.webworker.d.ts",
"lib.webworker.importscripts.d.ts",
"lib.webworker.iterable.d.ts"
] |
I've added code like: const files =
"getAllLibFileNames" in ts
? // @ts-ignore - see https://github.com/microsoft/TypeScript/pull/54011
(ts.getAllLibFileNames() as string[])
: [
"lib.d.ts",
"lib.decorators.d.ts",
"lib.decorators.legacy.d.ts",
"lib.dom.d.ts",
"lib.dom.iterable.d.ts", As that fn seems to be the one which is just the file name equivalents in your PR |
packages/typescript-vfs/src/index.ts
Outdated
"getAllLibFileNames" in ts | ||
? // @ts-ignore - see https://github.com/microsoft/TypeScript/pull/54011 | ||
(ts.getAllLibFileNames() as string[]) | ||
: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this list get filtered in any way to the actual set based on what's available? I'm not sure I totally know how this list is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there's a rough heuristic which cuts down the list based on target
+ lib
- just below:
const targetToCut = ts.ScriptTarget[target]
const matches = files.filter(f => f.startsWith(`lib.${targetToCut.toLowerCase()}`))
const targetCutIndex = files.indexOf(matches.pop()!)
const getMax = (array: number[]) =>
array && array.length ? array.reduce((max, current) => (current > max ? current : max)) : undefined
// Find the index for everything in
const indexesForCutting = lib.map(lib => {
const matches = files.filter(f => f.startsWith(`lib.${lib.toLowerCase()}`))
if (matches.length === 0) return 0
const cutIndex = files.indexOf(matches.pop()!)
return cutIndex
})
const libCutIndex = getMax(indexesForCutting) || 0
const finalCutIndex = Math.max(targetCutIndex, libCutIndex)
return files.slice(0, finalCutIndex + 1)
}
So the order is important WRT downloading which have some tests covering the cases, but the rough gist is that the CDN version will download all of the files (and cache them locally if possible) and put them into the VFS which TypeScript will then use synchronously later
I just added some checks for handle CDNs returning NOOPs for dts files which dont exist for that ts version
I haven't had time to revisit the TS API for this quite yet; in the meantime to fix everyone, maybe we'd be better off keeping that full list without checking that new API, and we can come back to adding a conditional use later? I'm hoping to get all of the PRs that need to be published to npm out somewhat soon. |
Yep, this PR keeps the full list and if an API called |
Right, I mean that I'm not sure that function is even the one that's going to go into TS itself, so I was thinking we'd just delete that reference for now. That and I want to double check that the files list is up to date for 5.3 which I'll do shortly. |
Here is an updated array of all lib.d.ts files to use in the PR:
I think for now, I'd just like to get this list in as-is, no reference to that new API, and we can add it later. That way, we can release this and fix things up for everyone. |
.then(contents => { | ||
contents.forEach((text, index) => fsMap.set("/" + files[index], text)) | ||
}) | ||
// Return a NOOP for .d.ts files which aren't in the current build of TypeScript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, looking back, is this right? Won't this fail if any of them fail?
With bump versions with #2904 to minimize the waiting within the current infrastructure. |
Apparently ordering matters to pass tests. Hopefully it's the test that's wrong because otherwise, it's going to be challenging to fix because it's not like an FS-based API from TS for this will be returning things in any particular order... |
Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-glacier-09aa73710-2802.centralus.azurestaticapps.net |
1 similar comment
Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-glacier-09aa73710-2802.centralus.azurestaticapps.net |
Azure Static Web Apps: Your stage site is ready! Visit it here: https://delightful-glacier-09aa73710-2802.centralus.azurestaticapps.net |
Yep, order is important to the implementation (which is why there's tests!): #2802 (comment) Looks right to me though |