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

Support for import('./native.node') #55821

Open
guybedford opened this issue Nov 11, 2024 · 3 comments · May be fixed by #55844
Open

Support for import('./native.node') #55821

guybedford opened this issue Nov 11, 2024 · 3 comments · May be fixed by #55844
Labels
addons Issues and PRs related to native addons. esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js.

Comments

@guybedford
Copy link
Contributor

What is the problem this feature will solve?

Currently require needs to be used in ES modules in order to be able to load native addons:

import { feature } from './native.node';

Previous issue: #40541.

There is some discussion to be had around whether import assertions should be used for the type. Personally I would even prefer if the import assertion wasn't necessarily mandated like we do for Wasm imports.

What is the feature you are proposing to solve the problem?

Transparently add support for import('./native.node') to work in the Node.js ESM implementation, based on the .node file extension.

This is one of the few last features that require() supports but import() does not, and would help gain full parity between the module systems.

What alternatives have you considered?

The alternative is to rely on process or have a more ergonomic process.getNativeModule('./mode.node'), but I think users would prefer the ergonomics of a direct import.

Some thought may need to be put to default versus named exports support in this interface as well.

@guybedford guybedford added the feature request Issues that request new features to be added to Node.js. label Nov 11, 2024
@github-project-automation github-project-automation bot moved this to Awaiting Triage in Node.js feature requests Nov 11, 2024
@aduh95
Copy link
Contributor

aduh95 commented Nov 11, 2024

The alternative is to rely on process or have a more ergonomic process.getNativeModule('./mode.node'), but I think users would prefer the ergonomics of a direct import.

That would either load ./mode.node as a path relative to the CWD (a bit confusing coming from require()), or would require passing a base URL (i.e. process.getNativeModule('./mode.node', import.meta.url))

@legendecas legendecas linked a pull request Nov 14, 2024 that will close this issue
@GeoffreyBooth GeoffreyBooth added addons Issues and PRs related to native addons. esm Issues and PRs related to the ECMAScript Modules implementation. labels Nov 14, 2024
@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Nov 14, 2024

cc @nodejs/loaders

+1 for this. In my mind the only question is whether or not to require the type attribute; see #40541 (comment) and #40766. Are there any spec concerns with leaving it off? If we eventually support non-experimental import of Wasm, will the attribute be required there? If so, it might seem odd that the attribute isn’t needed for .node while it is needed for .wasm and .json (and other non-JavaScript types like .css in browsers).

@guybedford
Copy link
Contributor Author

Wasm was landed in HTML recently without requiring the "type" attribute. So it's only needed for the "lesser capability" module formats of CSS and JSON.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addons Issues and PRs related to native addons. esm Issues and PRs related to the ECMAScript Modules implementation. feature request Issues that request new features to be added to Node.js.
Projects
Status: Awaiting Triage
Development

Successfully merging a pull request may close this issue.

4 participants
@GeoffreyBooth @guybedford @aduh95 and others