Use user's typescript first, fallback to bundled#741
Use user's typescript first, fallback to bundled#741developit merged 5 commits intodevelopit:masterfrom
Conversation
🦋 Changeset detectedLatest commit: 32d8056 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@whitetrefoil, could you explain a little what the origin is of this feature? I bet you want to use typescript 4.0 and we don't support it yet? |
Yes, you're correct. See #711 where has 2 examples there: BTW, IMO use the ts version of the project to be bundled should make more sense than using a fixed version. |
developit
left a comment
There was a problem hiding this comment.
Looks good! I think we can simplify this down to basically the following:
require(resolveFrom.silent(options.cwd, 'typescript') || 'typescript'))
... but otherwise good to go.
| import cssnano from 'cssnano'; | ||
| import { rollup, watch } from 'rollup'; | ||
| import builtinModules from 'builtin-modules'; | ||
| import resolveFrom from 'resolve-from'; |
There was a problem hiding this comment.
why do we need this instead of just require.resolve and will this break with something like yarn pnp / pnpm?
There was a problem hiding this comment.
Not much different in theory. "resolve-from" needs node v8.0 while paths options of require.resolve requires node v8.9. And "resolve-from" support .silent which will lead to a bit nicer code in this case.
This PR works with yarn, I'm using yarn actually. But I have no idea about pnpm.
BTW there's an issue in "require-resolve" which complained that paths of require.resolve doesn't work. But I never dug deeper about that.
There was a problem hiding this comment.
Node 8 is EOL so that doesn't matter
You're using yarn 2? Then it's fine to merge
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| 'microbundle': minor | |||
There was a problem hiding this comment.
Do you consider this a breaking change?
There was a problem hiding this comment.
Used to considered it to be a fix + feature instead of break, and it can fallback to previous behavior.
After thought more carefully... Maybe you're right, in theroy if user previously used different TS version during developing & bundling, it will lead to different result.
But I'm still hesitating. Logically if a user previously:
- used same version in dev & bundling - no change
- verA in dev + verB in bundling, both worked - use verA in both stages should also work
- verA in dev + verB in bundling, OK in dev but failed to bundle - this PR means a fix i.o. a break to this user
- verA in dev + verB in bundling, OK in bundling but failed in dev - this PR is a break, but I'm not sure if this just happen...
Anyway, major is always a safer choice... Shall I update the PR?
This PR is aimed at fixing #711.
Microbundle will load "typescript" from
path.dirname(process.argv[1]).If no
process.argv[1], or failed to resolve "typescript" from there, fallback to the bundled "typescript".P.S.
Using
resolve-from@v4.0.0instead of the latestv5.0.0becausev4.0.0has already been inpackage-lock.json.