Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Use user's typescript first, fallback to bundled #741
Conversation
|
| Name | Type |
|---|---|
| microbundle | Minor |
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. |
|
Looks good! I think we can simplify this down to basically the following:
... but otherwise good to go. |
| @@ -9,6 +9,7 @@ import autoprefixer from 'autoprefixer'; | |||
| import cssnano from 'cssnano'; | |||
| import { rollup, watch } from 'rollup'; | |||
| import builtinModules from 'builtin-modules'; | |||
| import resolveFrom from 'resolve-from'; | |||
ForsakenHarmony
Nov 10, 2020
Collaborator
why do we need this instead of just require.resolve and will this break with something like yarn pnp / pnpm?
why do we need this instead of just require.resolve and will this break with something like yarn pnp / pnpm?
whitetrefoil
Nov 10, 2020
Author
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.
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.
ForsakenHarmony
Nov 11, 2020
Collaborator
Node 8 is EOL so that doesn't matter
You're using yarn 2? Then it's fine to merge
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 | |||
ForsakenHarmony
Nov 11, 2020
Collaborator
Do you consider this a breaking change?
Do you consider this a breaking change?
whitetrefoil
Nov 12, 2020
Author
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?
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.