Repository navigation
Design Meeting Notes, 2/24/2023 #52953
Copy link
Copy link
Closed
Labels
Design NotesNotes from our design meetingsNotes from our design meetings
Description
Libraries Patching TypeScript's API
https://gist.github.com/jakebailey/3e73c7aab4da2044a95121467aa3b4e9
- In TypeScript 5.0, libraries can no longer override properties of our API.
- Language service plugins
- typescript-plugin-css-modules
- Can no longer patch
createLanguageServiceSourceFileandupdateLanguageServiceSourceFile - Make ones up with CSS.
- Should be proxying the language service host
- Can no longer patch
- vuejs/language-tools
- Same deal with
resolveModuleName- The new 5.0 resolution modes are better fits.
--moduleResolution bundler--allowArbitraryExtensions- Use
.d.vue.tsfiles.
- Use
- The new 5.0 resolution modes are better fits.
- There's really no host-level function they can override? Surprising.
- Seems like no?
- But resolveModuleName takes a
ModuleResolutionHost- is there a way to control that?- Seems fairly file-system-oriented.
- Actually some hosts do have a
resolveModuleNamesproxyPluginHostshould sethost.resolveModuleNameLiterals.
- Still, using the new resolution modes is recommended, but if they're determined to support node16/nodenext (they shouldn't, we don't believe it's actually the the right thing to use), patching
resolveModuleNameLiteralsis the right thing. - Meta: they recommend most users use this instead of our language server: https://vuejs.org/guide/typescript/overview.html#volar-takeover-mode
- Same deal with
- typescript-plugin-css-modules
- Supporting Transformers
- ttypescript
createProgram
- ts-patch
createProgram, lots of other top-level stuff to enable caching.
- The core idea is that these packages don't want to support all of the tsc command line and whatnot.
- Is
executeCommandLinepublic?- Yes. (later: no)
- Then they should be using that.
- Ehh, it's not exported.
- Only internally exported for testing scenarios.
- Also there's no emit hook. Can't plug transforms in today.
program.emitis overridable fromexecuteCommandLine- But that happens after emit occurs.
- But you can set
noEmitand then force emit. - But then you don't get declaration file diagnostics.
- Come back to these.
- ttypescript
- Stencil - has its own compiler
- Sets
ts.sys - We have an
/* @internal */ function setSysthat they can use. - Also use
resolveModuleName- they should be passing this into theCompilerHostthey create - So actually, most of this should be on the
CompilerHost - There are some functions they patch which we missed.
- Sets
- Heft
- Build system that wraps TypeScript.
readJson- only reason for this is caching.emitFiles- patched so that they can support multiple emit targets.- Possible idea: you have the
CompilerHost, just patch it every time and re-run emit.- Tried, but was unsuccessful.
- Yarn
- Still works! Patched in a different way, maintained in a fork.
- Would like to make this easier, but not in near-term range.
Comparing Wrapper Objects
1is anumberNumber(1)is anumbernew Number(1)is aNumber- a wrapper object.- All numeric and relational operations turn the wrapper objects back into their primitive equivalents.
===is suspect though.- In 5.0,
number < Numberis disallowed - We have a change that makes relational comparison consider
valueOf(Stricter relational comparisons checkingvalueOf#52807) - which re-allowsnumber < Number! - We don't feel like it's a huge problem to error on these - would prefer people not use these types.
- People shouldn't be referencing these anyway - default typescript-eslint rule is
ban-typeswhich stops these.- So not unbreaking
number < Numberin 5.0 - it's too late in the game regardless of what happens in 5.1.
- So not unbreaking
- Aside: probably should not perform these in the relationship checks, should liken these to the
NaNchecks.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Design NotesNotes from our design meetingsNotes from our design meetings
Activity
uasan commentedon Feb 25, 2023
To integrate modules that are not
.js, .jsx, .json, there are 2 options to create real-time d.ts files or hack system TS methods.We chose the second option, we overridden the
hostBuilder.resolveModuleNamesmethod for file observe of custom extensions (.css, .svg) andts.createSourceFile(I know this is a terrible hack) in this method for custom extensions we use the corresponding parsers and based on their AST we generate and return text d.ts.Creating d.ts files is much slower and triggers a lot of processes, git plugins, eslint, etc...
Options
--moduleResolution bundlerand--allowArbitraryExtensions, do require d.ts files, but we need a legal method in which we can get the contents of custom files and return d.ts text without creating d.ts files, perhaps this method already exists, but I don’t know?johnsoncodehk commentedon Feb 25, 2023
Vue also patched
supportedTSExtensions,supportedJSExtensions,allSupportedExtensions,createProgramviafs.readFileSyncfor tsc. (I think TS 5.0 can't prevent this. 👀)https://github.com/vuejs/language-tools/blob/04d84f604e0fbf025f76f58a46c75b59bc1ca57e/packages/vue-tsc/bin/vue-tsc.js#L13-L18
jakebailey commentedon Feb 25, 2023
For the LS plugin, you should be able to do everything you need by providing host methods.
I didn't find that vue-tsc, and that's really unfortunate. But, that's not a runtime patch, kinda like some of the other ones, and is already fragile as it is...
uasan commentedon Feb 25, 2023
The host does not have a method that is responsible for returning the synthetic content of files with a custom extension, which is why you have to do these dirty hacks.
If d.ts files are created synthetically in runtime, then it is more efficient not to create these files on the file system, you need to add a method to the host that can be overridden, which will return the generated content in the d.ts syntex for files with custom extensions.
jakebailey commentedon Feb 26, 2023
I'm a little confused; is
readFilenot enough?jakebailey commentedon Feb 26, 2023
In any case, this issue is not the right place for this sort of discussion; there's an issue for the css plugin (#52847), and I think it'd be best if there were a distinct issue for the APIs that you believe are missing too, so we can either track the addition, or commentary on a safer way to do the same thing.
uasan commentedon Feb 26, 2023
No readFile, only called for files that exist on the filesystem, we don't create files on the filesystem.
Ok, I'll open a new issue, thanks.
jakebailey commentedon Feb 26, 2023
Sorry, I should have expanded that more; readFile + the other FS related methods, like readDirectory and such too.
uasan commentedon Feb 26, 2023
Done
#52983
sminnee commentedon Nov 25, 2024
FYI
vue-tscbroke with Typescript 5.7.2 - vuejs/language-tools#5018 - it probably related to this, so just cross-linking here.jakebailey commentedon Nov 25, 2024
Everything here was related to TypeScript 5.0, I don't think it has anything to do with a change which happened in TypeScript 5.7