Skip to content
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: esm imports need js extension #34

Merged
merged 3 commits into from
Jun 21, 2021

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Jun 18, 2021

Because nodejs requires file extensions for relative imports, the current esm build fails to use in Node.js

As an example, importing packToBlob:

Error [ERR_UNSUPPORTED_DIR_IMPORT]: Directory import '/Users/vsantos/work/pl/gh/protocol/nft.storage/packages/client/examples/node.js/node_modules/ipfs-car/dist/esm/pack/' is not supported resolving ES modules imported from /Users/vsantos/work/pl/gh/protocol/nft.storage/packages/client/examples/node.js/node_modules/ipfs-car/dist/esm/pack/blob.js

This PR uses ttsc to rely on a custom transformer, which during the build adds the .js extension to relative imports. Note that index.js is not inferred and index needed to be added

Related:

@vasco-santos vasco-santos force-pushed the fix/esm-imports-need-js-extension branch from 858d279 to f94bb9f Compare June 20, 2021 15:37
@vasco-santos vasco-santos marked this pull request as ready for review June 20, 2021 15:47
@vasco-santos
Copy link
Member Author

This seems to fix the problem (tested in nft.storage client Node.js example)

@alanshaw
Copy link
Member

I think by spec you need the extension - why not just add it, instead of adding a plugin?

Incase you didn't know: you can import with .js in typescript and it'll do the right thing.

@vasco-santos
Copy link
Member Author

I think by spec you need the extension - why not just add it, instead of adding a plugin?

Incase you didn't know: you can import with .js in typescript and it'll do the right thing.

Yes, it compiles fine and the resulting build works. But, when using ts-node it fails as it does not find file.js.

As we also use ts-node for the tests, it will fail. I could change the test setup, but I am not really a fan of just using the .js extension in the code imports to solve something that is related to the build.

@alanshaw
Copy link
Member

Ahh ok gotcha. Plugin it is then!

@vasco-santos vasco-santos merged commit 5043bb5 into main Jun 21, 2021
@vasco-santos vasco-santos deleted the fix/esm-imports-need-js-extension branch June 21, 2021 07:15
@matheus23 matheus23 mentioned this pull request Jun 21, 2021
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants