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

utils: fix ESM import #26

Closed
wants to merge 1 commit into from
Closed

Conversation

ocavue
Copy link
Contributor

@ocavue ocavue commented Feb 9, 2024

Fixes an incorrect import for ESM.

@paulmillr
Copy link
Owner

the question is: why have all tests been passing?

@ocavue
Copy link
Contributor Author

ocavue commented Feb 9, 2024

I should able to answer this question but I'm now in the bed so I will check it out tomorrow.

@paulmillr paulmillr closed this Feb 9, 2024
@paulmillr
Copy link
Owner

0.5.1 is out

@ocavue
Copy link
Contributor Author

ocavue commented Feb 10, 2024

why have all tests been passing

When importing a relative ESM file, ESM always requires the file extension. For example, Node.js in CommonJS module accepts both require('./bar.js') and require('./bar'), but Node.js in ESM only accepts import './bar.js'.

The test (test/index.js) is a CommonJS file, so it won't fail when the file extension (.js) is missing.


To prevent this issue, the simplest solution would be changing the moduleResolution from bundler to Node16 in tsconfig.esm.json, and add src/package.json with the content as {"type": "module"}.

When using "moduleResolution": "Node16", tsc would output CommonJS or ESM code based on the type field in the nearest package.josn. If the type is "module", then tsc would generate ESM code and also check the file extension, described above.

Another solution would be using a bundler to handle the rewrite of file extension. This requires more change though.

@ocavue
Copy link
Contributor Author

ocavue commented Feb 10, 2024

Update: I've submitted a PR to set modeResolution to Node16.

#27

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.

2 participants