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

Fixed missing .js to imports #25

Closed
wants to merge 6 commits into from

Conversation

BlobMaster41
Copy link

In Node.js, if you're working with ES Modules (.mjs files or .js files with "type": "module" in your package.json), you are required to include the .js extension in your import paths. Unfortunately, Node.js strictly enforces this rule for ES Modules, and there's no built-in way to make Node.js automatically resolve imports without the .js extension in this mode.

If you create a typescript project in mode module and try to use this package, you will get the following error:

Cannot find module 'node_modules\ecpair\src\esm\ecpair' imported from node_modules\ecpair\src\esm\index.js

In Node.js, if you're working with ES Modules (.mjs files or .js files with "type": "module" in your package.json), you are required to include the .js extension in your import paths. Unfortunately, Node.js strictly enforces this rule for ES Modules, and there's no built-in way to make Node.js automatically resolve imports without the .js extension in this mode.
Copy link
Member

@junderw junderw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments. Also, please add a test of some sort.

Ideally a bash script or something that makes a simple project in a temp folder that just tries to import this repository (via file install) and fails to build.

I will verify that it fails on my PC, and that this PR doesn't fail.

Then we can have it run in CI to make sure we don't mess up the hybrid package when making changes.

@@ -1,6 +1,6 @@
{
"name": "ecpair",
"version": "3.0.0-rc.0",
"version": "3.0.1-rc.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3.0.0-rc.1

@@ -0,0 +1,5 @@
# Default ignored files
/shelf/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't commit .idea folder please

@junderw
Copy link
Member

junderw commented Oct 18, 2024

There's a broken lint in the tests... also we should update the node version used in CI to something that has the crypto API (to stop the errors)

@btc-vision btc-vision closed this by deleting the head repository Dec 7, 2024
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.

3 participants