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

Customization hooks / ESM loaders reports importing JSON warning on commonjs modules #53318

Open
hyrious opened this issue Jun 4, 2024 · 2 comments

Comments

@hyrious
Copy link

hyrious commented Jun 4, 2024

Version

v22.1.0

Platform

Darwin MAGIT00999.local 23.5.0 Darwin Kernel Version 23.5.0: Wed May 1 20:09:52 PDT 2024; root:xnu-10063.121.3~5/RELEASE_X86_64 x86_64

Subsystem

No response

What steps will reproduce the bug?

Prepare the 3 files:

// register.mjs
import { register } from 'module'
import { readFile } from 'fs/promises'

export async function load(url, context, nextLoad) {
  const result = await nextLoad(url, context)
  if (result.format === 'commonjs') {
    // https://nodejs.org/api/module.html#:~:text=result.source%20%3F%3F%3D%20await%20readFile(new%20URL(result.responseURL%20%3F%3F%20url))%3B
    result.source ??= await readFile(new URL(result.responseURL ?? url))
  }
  return result
}

register('./register.mjs?' + Date.now(), import.meta.url)

// test.js
require('./package.json')

// package.json
{}

Then execute:

> node --import ./register.mjs test.js
(node:5296) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

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

It happens only with:

result.source ??= await readFile(new URL(result.responseURL ?? url));

If it does not handle commonjs files like this, it won't happen.

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

require(json_file) in commonjs modules should not emit this warning.

What do you see instead?

It emits warning

(node:5296) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time

Additional information

No response

@benjamingr
Copy link
Member

@nodejs/loaders @joyeecheung

@joyeecheung
Copy link
Member

Likely a regression from #47999

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

No branches or pull requests

3 participants