node icon indicating copy to clipboard operation
node copied to clipboard

ExperimentalWarning: Importing JSON modules

Open webdevnerdstuff opened this issue 1 year ago β€’ 10 comments

Version

20.9.0 & 20.10.0

Platform

Darwin foobar.local 22.6.0 Darwin Kernel Version 22.6.0: Thu Nov 2 07:43:57 PDT 2023; root:xnu-8796.141.3.701.17~6/RELEASE_ARM64_T6000 arm64

Subsystem

No response

What steps will reproduce the bug?

  1. Clone repo https://github.com/webdevnerdstuff/vuetify-drilldown-table/tree/dev
  2. git checkout dev
  3. pnpm i
  4. npx cross-env NODE_OPTIONS="--trace-warnings" pnpm test:dev (same result with npm run test:dev)
  5. Notice the console is throwing a warning that seems unrelated to the actual code in the project.

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

Every time you run a package script it's throwing this warning in the console.

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

It should not be throwing a warning if there is nothing that is being used in that warning. As it seems the trace shows the warnings coming from node:internal.

What do you see instead?

(node:12542) ExperimentalWarning: Importing JSON modules is an experimental feature and might change at any time
    at emitExperimentalWarning (node:internal/util:275:11)
    at ModuleLoader.jsonStrategy (node:internal/modules/esm/translators:453:3)
    at callTranslator (node:internal/modules/esm/loader:285:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:291:30)

Additional information

There are places in the code where I am importing a json file, but I also tested by removing all of those imports, and the output of the warning is the same. So I'm not sure where and/or why it's throwing this warning. Is this a false positive?

webdevnerdstuff avatar Jan 03 '24 04:01 webdevnerdstuff

Well unless you're able to reproduce without using any external dependency, I'm going to assume one of your deps is importing a JSON module, which is still experimental hence the runtime warning you're seeing. You can add NODE_DEBUG=esm to your env to get more information on what is being imported at the time the warning gets emitted.

aduh95 avatar Jan 03 '24 13:01 aduh95

There are a few problems with how the warning is being handled currently in my opinion that could use improvements. I now realize it is the warning itself that is the issue here.

Not using any external dependencies is not a helpful response as that is not a realistic or real world situation, nor does it help solve the issue, as removing packages would just cause an avalanche of other errors. Your first sentence also comes across as a bit condescending, but I'm just going to assume it's a language barrier situation and move on.

I think it would be more useful if the warning mentioned that it could be an external dependency causing the issue. I spent a good amount of time combing through my own code looking in the wrong direction trying to figure out what may have caused the warning. If in a larger team, it could cause multiple people looking in the wrong direction and wasting a lot of time as well.

It think it would also be helpful if the warning included your suggestion to use NODE_DEBUG=esm, which is a much better suggestion to figure out the problem. The message given includes:

(Use `node --trace-warnings ...` to show where the warning was created)

When you run that command, it makes it seem like it's a node problem, not any external dependency. Perhaps I read the warning wrong at first, but it's message implies it will help find what's causing the warning in my code. Another thing it could imply is that it will just tell you where in the node source code that is throwing that warning, which is completely useless when problem solving what's causing the warning in the project. I want to know what's causing the issue so I can fix it and/or perhaps find another solution in the dependency causing it, not what code is throwing the warning within node as that is useless information, unless I'm debugging the actual warning message itself.

I would ask to the node team to make some adjustments to the warning to help avoid a lot of wasted time spent trying to find an issue while looking in the wrong direction. It could save time spent debugging and potentially an entire team doing the same, searching the internet for a solution, submitting an issue to GitHub, reading the response, spending time responding, and would have saved any Contributor to node, time reading/responding to the submitted issue.

webdevnerdstuff avatar Jan 03 '24 19:01 webdevnerdstuff

I can accept that it's experimental, but is there a way to suppress the warning?

johnleider avatar Jan 03 '24 20:01 johnleider

With older versions, --no-warnings will turn off all warnings. Newer versions also support --disable-warning={code} where {code} is the specific code associated with an individual warning (if there is one)

jasnell avatar Jan 03 '24 20:01 jasnell

@jasnell It looks like that was added in v21.3.0. That would be nice to have in the LTS version.

For now I'll most likely just add npx cross-env NODE_OPTIONS='--no-warnings' (or just NODE_OPTIONS='--no-warnings' depending) into the script.

webdevnerdstuff avatar Jan 03 '24 20:01 webdevnerdstuff

Your first sentence also comes across as a bit condescending, but I'm just going to assume it's a language barrier situation and move on.

Apologies, that was not my intention. I was meant as an answer for β€œis this a false positive?”

For now I'll most likely just add npx cross-env NODE_OPTIONS='--no-warnings' (or just NODE_OPTIONS='--no-warnings' depending) into the script.

Note that disabling warnings means you’re more likely to miss important information, you might want to consider avoiding use of experimental API instead (or accept to see the warning). If you are writing something that is going to be run by someone else, have in mind that removing the warning might be a disservice to your users, as experimental APIs are not bound to semver rules and could break anytime they update their version of Node.js.

aduh95 avatar Jan 03 '24 22:01 aduh95

Apologies, that was not my intention. I was meant as an answer for β€œis this a false positive?”

I had a feeling it was just a language difference in how the sentence came across. No worries.

In my situation where this came up, it is a component for another library. So it's unlikely this will affect them when it comes to working on this component, which I don't anticipate anybody wanting to help with this one. The package that's causing the warning is not something I can do without (at least not without difficulty), and/or would be used by the users in their own project as well, so as long as they don't silent the warning, they will still get it.

Which brings me back to the message could use a little bit of tweaking in it's wording to help avoid confusion for other users when this comes up.

webdevnerdstuff avatar Jan 03 '24 23:01 webdevnerdstuff

Not sure how I feel about promoting NODE_DEBUG usage in such warning, it's meant as a tool for Node.js core devs, not for end-users. Usually --trace-warnings gives back a useful stacktrace, the unfortunate reason it's rather useless here is because import is not a function, it's a language construct, and therefore does not appear in stack traces. I remember a V8 ticket was opened regarding trying to improve that, but I couldn't find it again.

We could try to add the information to the warning of which import triggered the warning, PRs welcome.

In the mean time, I think you should report to the dependency you are using that they should document they use experimental syntax, and that users should expect to see that warning, and what that means in terms of support from Node.js updates (and how to silence the warning if that's what you want).

aduh95 avatar Jan 04 '24 11:01 aduh95

Adding which import triggered the warning sounds like an excellent idea as that would get straight to the source of the problem. I wouldn't know where to begin to even attempt to submit a PR for it. If/when I get some free time, I'll definitely look into it and give it a try if someone else doesn't get to it first.

I somewhat agree about promoting NODE_DEBUG, as that could confuse people who are not as technical and overwhelm them with the output it gives not knowing what they should be looking for.

At the very least, I think adding a message saying it could be an external dependency would be very helpful so people can possibly avoid having to dig through potentially hundreds of files looking for something that might not exist in their own code.

webdevnerdstuff avatar Jan 04 '24 19:01 webdevnerdstuff

I add a shebang like this to make the bin script working without showing the warnings. And it works for windows too.

#!/usr/bin/env node --no-warnings=ExperimentalWarning

liudonghua123 avatar Jan 16 '24 05:01 liudonghua123

@liudonghua123 thanks for that workaround

The clusterfck that is ESM in Node never ceases to amaze.

shellscape avatar May 11 '24 15:05 shellscape

@shellscape I did the following to work around the warning.

import { createRequire } from "module";
const PJSON = createRequire(import.meta.url)("./package.json");

petersem avatar May 14 '24 23:05 petersem