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

Import from JSON data URL doesn't work #37647

Closed
evanw opened this issue Mar 7, 2021 · 2 comments · Fixed by #37701
Closed

Import from JSON data URL doesn't work #37647

evanw opened this issue Mar 7, 2021 · 2 comments · Fixed by #37701
Labels
esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@evanw
Copy link

evanw commented Mar 7, 2021

  • Version: v15.11.0
  • Platform: Darwin Evans-MacBook-Pro.local 19.6.0 Darwin Kernel Version 19.6.0: Tue Jan 12 22:13:05 PST 2021; root:xnu-6153.141.16~1/RELEASE_X86_64 x86_64
  • Subsystem: ?

What steps will reproduce the bug?

To reproduce, run this code in an ECMAScript module:

import 'data:text/javascript,console.log("hello!");';
import _ from 'data:application/json,"world!"';

The code will crash immediately. This code is taken verbatim from node's documentation: https://nodejs.org/docs/latest/api/esm.html#esm_data_imports. The relevant part of the documentation is pasted below for reference:

data: Imports

Added in: v12.10.0

data: URLs are supported for importing with the following MIME types:

  • text/javascript for ES Modules
  • application/json for JSON
  • application/wasm for Wasm

data: URLs only resolve Bare specifiers for builtin modules and Absolute specifiers. Resolving Relative specifiers does not work because data: is not a special scheme. For example, attempting to load ./foo from data:text/javascript,import "./foo"; fails to resolve because there is no concept of relative resolution for data: URLs. An example of a data: URLs being used is:

import 'data:text/javascript,console.log("hello!");';
import _ from 'data:application/json,"world!"';

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

Always.

What is the expected behavior?

The snippet should print hello! and exit. Presumably if you add console.log(_) it should print world! as well.

What do you see instead?

node:internal/process/esm_loader:74
    internalBinding('errors').triggerUncaughtException(
                              ^

TypeError [ERR_INVALID_RETURN_PROPERTY_VALUE]: Expected string to be returned for the "format" from the "loader getFormat" function but got type object.
    at new NodeError (node:internal/errors:329:5)
    at Loader.getFormat (node:internal/modules/esm/loader:111:13)
    at async Loader.getModuleJob (node:internal/modules/esm/loader:231:20)
    at async ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:59:21)
    at async Promise.all (index 1)
    at async link (node:internal/modules/esm/module_job:64:9) {
  code: 'ERR_INVALID_RETURN_PROPERTY_VALUE'
}

This error message goes away when the application/json import is removed.

Additional information

I am logging this because I am trying to understand if this is supposed to work. If not, the documentation probably just needs to be updated. But if it is supposed to work, it would be good to know if the example code needs to be updated. For context I'm the main developer of the esbuild bundler and I'm trying to have it match node's behavior here, so I need to know what the behavior is supposed to be.

@targos targos added the esm Issues and PRs related to the ECMAScript Modules implementation. label Mar 7, 2021
@targos
Copy link
Member

targos commented Mar 7, 2021

It's expected that this code doesn't work without the --experimental-json-modules flag. However, a crash like this instead of a clear error message is probably a bug.

@benjamingr
Copy link
Member

@nodejs/modules also #37375

aduh95 added a commit to aduh95/node that referenced this issue Mar 10, 2021
aduh95 added a commit to aduh95/node that referenced this issue Mar 10, 2021
@aduh95 aduh95 closed this as completed in 46062a0 Apr 3, 2021
MylesBorins pushed a commit that referenced this issue Apr 4, 2021
Fixes: #37647

PR-URL: #37701
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
MylesBorins pushed a commit that referenced this issue Apr 5, 2021
Fixes: #37647

PR-URL: #37701
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
targos pushed a commit that referenced this issue May 1, 2021
Fixes: #37647

PR-URL: #37701
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@evanw @benjamingr @targos and others