-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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/typescript 4 node module resolution #1764
base: main
Are you sure you want to change the base?
Fix/typescript 4 node module resolution #1764
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hi @nyan-left! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
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.
The .js
renaming seems to break resolution in the website build when Webpack tries to find nonexistent .js file.
Module not found: Error: Can't resolve './wrapAssembly.js' in '/vercel/path0/javascript/src'
Also seems to break resolution in test runner.
Let's hold off on doing anything too inorganic to support TypeScript < 5.0, since it's no longer in support.
These two are resolved with a bit of config for jest and docusaurus/webpack. |
Regarding linting, given the small nature of the I'm not too keen on hard coding the tsconfig paths, and afaik a more modular approach like this doesn't currently exist. |
Changing the resolvers in each of these is clever 🙂. It is also the kind of debt that I would like to avoid for TS versions out of support. It means that, any time in the future, if someone adds a I'm less concerned with the changes to avoid the need for ESM resolution, since they are still relevant in supported TS versions, and are less likely to break with future code changes. Re ESLint, we could specialize this specific file in the ESLint config, or we could maybe keep everything in the same project, and just add a step to copy the declaration for this entrypoint? |
I think we can solve this via Regarding resolvers, it's the reccommended approach by the webpack team. Looks like the only reason it's not the default in webpack already is because webpack doesn't support ts "out of the box". Perhaps one to reporo and make an issue with docusaurus to see their stance. webpack/webpack#13252 (comment) Jest suggest using |
Moving the load.d.ts works for me! :) |
Hi @NickGerleman, I'm wondering if you have had any further thoughts regarding the |
Closes #1760
This PR adds support for TypeScript 4.x and
"node"
module resolution by fixing import extension handling. Previously, the package only worked with TypeScript 5.x due to.ts
extensions in declaration files causingTS2691
errors in earlier versions.Changes
.ts
to.js
, configured in:Jest
-moduleNameMapper
Docusaurus/Webpack
-extensionAlias
tsconfig.load.json
for the/load
entry pointjust
build process:load.ts
imports to point todist/src
package.json
to includeload.*
files