[URGENT] ESM import JSON not working (devel only)

[advaiyalad]

Bug Report

This is urgent, as it essentially breaks everything. Yes, every last usage of this library (that doesn't use the CJS fallback, more on that later) is broken when using devel code. Luckily, the ESM code hasn't been released yet on NPM.

Describe the bug

When using any part of the library that makes a network request, CLI, or otherwise, an error pops up:

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

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".json" for [some_path]\node-yahoo-finance2\dist\esm\package.json
←[90m    at Loader.defaultGetFormat [as _getFormat] (internal/modules/esm/get_format.js:71:15)←[39m
←[90m    at Loader.getFormat (internal/modules/esm/loader.js:102:42)←[39m
←[90m    at Loader.getModuleJob (internal/modules/esm/loader.js:231:31)←[39m
←[90m    at async ModuleWrap.<anonymous> (internal/modules/esm/module_job.js:58:21)←[39m
    at async Promise.all (index 2)
←[90m    at async link (internal/modules/esm/module_job.js:63:9)←[39m {
  code: ←[32m'ERR_UNKNOWN_FILE_EXTENSION'←[39m
}

Minimal Reproduction

await yahooFinance.quote('AAPL') or any other module that makes a network request (once you have linked the project to use devel code)

Environment

Browser or Node: Node
Node version (if applicable): 14.16.1
Npm version: 6.14.12
Browser version (if applicable): n/a
Library version (e.g. 1.10.1): a78dd72

Comments and Solutions

Reason for issue

The reason behind this is that the ESM version, though compiled with the resolveJsonModule flag set to true, still imports the json module at runtime. Only require() can import JSON modules in node, and when this library was transpiled to CJS, the JSON file would be required. This is no longer the case, causing severe issues.

First way to solve the problem (preferred)

Can be solved by removing/hardcoding the user agent in yahooFinanceFetch.ts. This may be a breaking change though. This would keep all usage of the library the same.

The rest of the ways will break browser usage, just keeping them there in case somebody comes up with these ideas.

The second way to solve this problem

Force everyone to use --experimental-json-modules when using this lib. When nodejs/node#37375 is merged, then force everyone to either use the flag or use node 16. I am pretty sure that this would break most cases of browser usage.

The third way to solve this

Use fs.readFile and read the json file instead of importing. This would break browser usage, though.

The fourth way to solve this

Use module.createRequire() and require the file from ESM. This would also break browser usage.

@gadicc What is your opinion? Will removing the UA be a breaking change. or not?

[gadicc]

Hi, yes, I mention --experimental-json-modules in the README in the ES Modules section, but I must admit, I didn't think about the browser.

Note: it's not just package.json, we need schema.json for the library to work.

I had been thinking of some alternatives, since of course it would be nice to use no flags at all with later node versions.

Best case scenario is if typescript solves this for us, but the other thing I was thinking of was to just transpile the json to .js files at build time. It's a bit of schlepp though:

• Use real json import in testing
• Build step
  • Change import in all files to import .js rather
  • Transpile JSON to js
    • as module.exports in dist/cjs
    • as default export in dist/esm

There's also an issue with the "built" package.json files getting the wrong { "version" } value, since the build happens before publish time when the correct version is put there, but that's an issue with typescript's json module stuff too. Will have to see if there's a semantic-release pre-publish hook to fix that.

[gadicc]

🎉 This issue has been resolved in version 1.11.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀