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

Node does not determine that a file is an ES module when the package.json is in a long path #54304

Closed
BalaM314 opened this issue Aug 10, 2024 · 4 comments · May be fixed by #54316
Closed

Node does not determine that a file is an ES module when the package.json is in a long path #54304

BalaM314 opened this issue Aug 10, 2024 · 4 comments · May be fixed by #54316
Labels
loaders Issues and PRs related to ES module loaders

Comments

@BalaM314
Copy link

BalaM314 commented Aug 10, 2024

Version

22.6.0

Platform

Microsoft Windows NT 10.0.22631.0 x64

Subsystem

No response

What steps will reproduce the bug?

Create a .js file with the following code:

console.log("Hello world!");
export {};

and a package.json file with the following code:

{
    "type": "module"
}

in a directory with path length greater than 260 characters

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

Node should look at the closest package.json, see "type": "module", determine that the file is an ES module, and run it

What do you see instead?

Node is unable to read the package.json, defaults to CommonJS, and fails at export {}

Additional information

Showcase

@RedYetiDev
Copy link
Member

RedYetiDev commented Aug 10, 2024

# repro.sh
prefix=$(printf 'a%.0s' {1..128})

# Remove directory if it exists, then recreate it
rm -rf "$prefix" && mkdir -p "$prefix/$prefix"

# Create index.js and package.json in the nested directory
echo "export 'Loaded!';" > "$prefix/$prefix/index.js"
echo '{"type":"module"}' > "$prefix/$prefix/package.json"

# Run the Node.js script to import the module and log the output
node -p "import('./$prefix/$prefix/index.js').then(console.log)"
$ bash repro.sh
Promise {
  <rejected> SyntaxError: Unexpected token 'export'
      at compileSourceTextModule (node:internal/modules/esm/utils:337:16)
      at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:166:18)
      at callTranslator (node:internal/modules/esm/loader:436:14)
      at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:442:30)
      at async ModuleJob._link (node:internal/modules/esm/module_job:106:19)
}
file:///<...>/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/index.js:1
export 'Loaded!';
^^^^^^

SyntaxError: Unexpected token 'export'
    at compileSourceTextModule (node:internal/modules/esm/utils:337:16)
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:166:18)
    at callTranslator (node:internal/modules/esm/loader:436:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:442:30)
    at async ModuleJob._link (node:internal/modules/esm/module_job:106:19)

Node.js v22.6.0

@anonrig
Copy link
Member

anonrig commented Aug 11, 2024

We probably miss ToNamespacedPath call somewhere

@BalaM314
Copy link
Author

BalaM314 commented Aug 11, 2024

First time contributing to node, or any C++ project

I've tried to fix this, and here's what I found:
Problem is likely in src/node_modules.cc at the function BindingData::GetPackageScopeConfig
There isn't a comment explaining what this function does, but I think it's this:

C++ function that accepts a JS string (URL of directory, should be dir\ or dir\file, not dir)
Throws a JS error if the URL is invalid
Throws a JS error if URL\package.json is invalid
If the path ends with node_modules, or the search fails, return a JS string with the last attempt for path to package json (even if nonexistent)
Find file_path of the package.json as a std::string
Returns GetPackageJSON(file_path)->Serialize() as a js value

The function uses the URL parser to go to the parent path for some reason, and there is a comment saying it should not do that
Also, the function doesn't call ToNamespacedPath, which @anonrig said was the likely cause of this issue

I rewrote the function to use std::filesystem::path instead of the URL parser and it seems to work
The function ToNamespacedPath requires a BufferValue, which requires a JS string with the path in it, but we don't have that because this function takes a URL as the js argument, not a path
I found some comments saying that the function ToNamespacedPath should also support being called with a string, should I do that?

@BalaM314
Copy link
Author

BalaM314 commented Sep 19, 2024

Fixed in latest 22.9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants