-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
fix: overwrite nodenext option when transpiling #11092
Conversation
transpileModule treats NodeNext as CommonJS because it doesn't read the package.json. Therefore we need to override it. Also see microsoft/TypeScript#53022 (the filename workaround doesn't work). Fixes #11086
🦋 Changeset detectedLatest commit: 9162fdd 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 |
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.
thank you! I was just coming to a similar conclusion, but you beat me to it 😄
the other thing I've been wondering is if we should switch nodenext
for node16
. it seems a bit brittle to use nodenext
as its meaning could change at any time and so i'm not sure if it's what we should encourage
I think it's better to use nodenext because it means we're always up to date with regards to how module resolution is done. |
Actually, on giving this more thought, I think there's another issue with the way we're testing this. We seem not to be using the It feels to me like we should delete the And then we should update the |
We should keep the tests, because it ensures that tsconfig settings we had a various points in time all work properly. The idea of having a "use create-svelte's current tsconfig" test is good. |
Oh, yeah, that makes sense. I didn't realize this was a |
transpileModule treats NodeNext as CommonJS because it doesn't read the package.json. Therefore we need to override it. Also see microsoft/TypeScript#53022 (the filename workaround doesn't work). Fixes #11086
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.