Skip to content

Fix import.meta.dirname in node 18 #23407

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

Closed
wants to merge 1 commit into from
Closed

Conversation

kripken
Copy link
Member

@kripken kripken commented Jan 14, 2025

Likely I am doing something wrong, but I need this change, or else this happens for me locally with node v18.20.1:

$ ./emcc test/hello_world.c
node:internal/errors:496
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:405:5)
    at validateString (node:internal/validators:162:11)
    at Module.join (node:path:1171:7)
    at find (file:///emscripten/src/utility.mjs:235:27)
    at read (file:///emscripten/src/utility.mjs:229:20)
    at file:///emscripten/src/compiler.mjs:30:34
    at ModuleJob.run (node:internal/modules/esm/module_job:195:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:337:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:106:12) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Node.js v18.20.1
emcc: error: 'nodejs emscripten/src/compiler.mjs /tmp/tmp9wv0eyav.json' failed (returned 1)

Clearing the cache, running bootstrap, npm ci etc., nothing works except for this patch.

@kripken kripken requested a review from sbc100 January 14, 2025 22:19
@sbc100
Copy link
Collaborator

sbc100 commented Jan 14, 2025

No, you are not doing anything wrong. This was an unfortunate side effect of #23349. I accidentally caused the compiler to start requiring node v19.

See #23396

emsdk was updated to v20 a while back now so it went unnoticed.

How did you end up with v18 on your system? Is it an old emsdk version maybe? Maybe you also forgot to do emsdk activate latest?

@kripken
Copy link
Member Author

kripken commented Jan 14, 2025

This is just the system node. I have it set to be used in my .emscripten file I guess, from a long time ago.

This does seem to fix things for me. Can we land it, or does it have downsides?

@sbc100
Copy link
Collaborator

sbc100 commented Jan 14, 2025

The right way to revert this would be to restore this part of the code to how it was before #23349.

@kripken
Copy link
Member Author

kripken commented Jan 14, 2025

Ok, I'll switch to 20 locally then.

@kripken kripken closed this Jan 14, 2025
@kripken kripken deleted the wat branch January 14, 2025 22:48
@sbc100
Copy link
Collaborator

sbc100 commented Jan 14, 2025

No, we should still support v18 if its the one in debian/stable. Working on anther change now..

sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 14, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.20.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 14, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.20.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 14, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.20.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 14, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.20.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 14, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.20.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 14, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 14, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 15, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit to sbc100/emscripten that referenced this pull request Jan 15, 2025
In emscripten-core#23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts emscripten-core#23349 and also explicitly bumps the minimum version
up to v18.  I've also added some testing to CI to ensure we can
actually run using this version.

See emscripten-core#23396 and emscripten-core#23407
sbc100 added a commit that referenced this pull request Jan 15, 2025
In #23349 we (I) accidentally broke running emscripten with node older
than v19 and as it happens debian/stable still ships v18:
https://packages.debian.org/bookworm/nodejs

This change reverts #23349 and also explicitly bumps the minimum version
up to v18. I've also added some testing to CI to ensure we can actually
run using this version.

See #23396 and #23407
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