feat(typescript): better error when tslib is not installed #793
Conversation
shellscape
force-pushed
the
feat/typescript/error-no-tslib
branch
6 times, most recently
from
5fcec69 to
fc3e53b
Compare
| } | ||
|
|
||
| /** Properties of `CompilerOptions` that are normally enums */ | ||
| export type EnumCompilerOptions = 'module' | 'moduleResolution' | 'newLine' | 'jsx' | 'target'; | ||
|
|
||
| /** JSON representation of Typescript compiler options */ | ||
| export type JsonCompilerOptions = Omit<CompilerOptions, EnumCompilerOptions> & | ||
| export type JsonCompilerOptions = Omit<FlexibleCompilerOptions, EnumCompilerOptions> & |
There was a problem hiding this comment.
The type changes here appear unnecessary, and throws out a lot of type-checking for typos. I'd rather leave it as is.
There was a problem hiding this comment.
Unfortunately it necessary for everything to play nicely together. The tests weren't written in TypeScript, and as soon as I started passing tslib as an option in a TypeScript test, it started throwing typing errors. Going to have to overrule you on this one, unless you can get the types playing nice in a more visually pleasing manner.
At issue is the way you typed the tslib option in the plugin options, and the way that CompilerOptions exposes the same property through the [options: string] index. They're incompatible.
There was a problem hiding this comment.
OK, I can always try taking a look at this again in a future PR
There was a problem hiding this comment.
Sorry for laying that out tersely, super tired over here. The blame is not yours, so didn't mean to imply that. Just how the types ended up, we got that funky collision. We'd probably have to use an alternate option name like tslibPath or something.
shellscape
force-pushed
the
feat/typescript/error-no-tslib
branch
from
fc3e53b to
8dfbf7f
Compare
shellscape
force-pushed
the
feat/typescript/error-no-tslib
branch
from
8dfbf7f to
9c127a6
Compare
Rollup Plugin Name:
typescriptThis PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers: Fixes #598
Description
This PR adds a meaningful error for users that addresses #598. Of note, this switches the use of the async
resolvemethod toresolve.sync. It was just too problematic dealing with promises and getting the error out of the promise chain and back out to the user properly blocking the build. That may actually be an issue with plugin architecture, but we have an easy workaround for it now. Testing tslib being completely missing was a challenge, and I ultimately resorted to a janky envar for simulating that (notes in code comments) after trying a dozen or so methods for removing it (or simulating it being gone) from local node_modules.