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

[Bug?]: PnP loader cannot handle jsons #4245

Closed
1 task done
espoal opened this issue Mar 18, 2022 · 9 comments · Fixed by #4786
Closed
1 task done

[Bug?]: PnP loader cannot handle jsons #4245

espoal opened this issue Mar 18, 2022 · 9 comments · Fixed by #4786
Assignees
Labels
bug Something isn't working esm upholded Real issues without formal reproduction

Comments

@espoal
Copy link

espoal commented Mar 18, 2022

Self-service

  • I'd be willing to implement a fix

Describe the bug

The pnp module loader seems incapable of loading jsons:

file:///home/mamluk/3pass/esm-pwa/.pnp.loader.mjs:125
      throw new Error(`Unknown file extension ".json" for ${filepath}`);
            ^

Error: Unknown file extension ".json" for /home/mamluk/3pass/esm-pwa/package.json
    at getFileFormat (file:///home/mamluk/3pass/esm-pwa/.pnp.loader.mjs:125:13)
    at load$1 (file:///home/mamluk/3pass/esm-pwa/.pnp.loader.mjs:174:18)
    at ESMLoader.load (node:internal/modules/esm/loader:407:26)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:326:22)
    at new ModuleJob (node:internal/modules/esm/module_job:66:26)
    at ESMLoader.#createModuleJob (node:internal/modules/esm/loader:345:17)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:304:34)
    at async ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:82:21)
    at async Promise.all (index 2)
    at async link (node:internal/modules/esm/module_job:87:9)

Node.js v17.7.2

To reproduce

test.mjs

import pkg from './package.json' assert { type: 'json' }
console.log({version: pkg.version})

With node test.mjs I get:

node test.mjs 
{ version: '0.0.1' }
(node:60949) ExperimentalWarning: Importing JSON modules is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

With yarn node test.mjs I get the error.

Environment

System:
    OS: Linux 5.13 Ubuntu 20.04.4 LTS (Focal Fossa)
    CPU: (12) x64 AMD Ryzen 5 3600 6-Core Processor
  Binaries:
    Node: 17.7.2 - /tmp/xfs-68437570/node
    Yarn: 3.2.0 - /tmp/xfs-68437570/yarn
    npm: 8.5.2 - /usr/bin/npm

Additional context

My .yarnrc.yml is:

yarnPath: .yarn/releases/yarn-3.2.0.cjs

pnpEnableEsmLoader: true
@espoal espoal added the bug Something isn't working label Mar 18, 2022
@espoal espoal changed the title [Bug?]: [Bug?]: PnP loader cannot handle jsons Mar 18, 2022
@merceyz merceyz added esm upholded Real issues without formal reproduction labels Mar 18, 2022
@CarlosBalladares
Copy link

CarlosBalladares commented Jul 4, 2022

Reproducible on yarn 3.2.1
image
node 16.14.0
even without the flag in the command still repro

@CarlosBalladares
Copy link

CarlosBalladares commented Jul 4, 2022

Example repo with bug
https://github.com/CarlosBalladares/yarn-pnp-loader-json-bug

To reproduce

  1. clone the repo https://github.com/CarlosBalladares/yarn-pnp-loader-json-bug
  2. run yarn && yarn workspace pkg-1 dev
  3. Observe

To run the same without error

  1. clone the repo https://github.com/CarlosBalladares/yarn-pnp-loader-json-bug
  2. run yarn && cd packages/pkg-1 && node --experimental-json-modules src/index.js
  3. Observe

Expected outcome:
Both test runs should print pkg-1

Actual outcome:
terminal output indicating error for the one using yarn to run the command

@samuelexferri
Copy link

samuelexferri commented Jul 27, 2022

My duplicate issue #4681

I'm trying to import into a .ts file a json file using Node v18 and --experimental-json-modules flag like this:

import genericERC20 from '../json/FILE.json' assert {type: "json"};

But when I try to run the script, I obtain this error:

You have triggered an unhandledRejection, you may have forgotten to catch a Promise rejection:
Error: Unknown file extension ".json" for /Users/samuelexferri/GITHUB/PROJECT/dist/json/FILE.json
    at getFileFormat (file:///Users/samuelexferri/GITHUB/PROJECT/.pnp.loader.mjs:129:13)
    at load$1 (file:///Users/samuelexferri/GitHub/PROJECT.pnp.loader.mjs:177:18)
    at ESMLoader.load (node:internal/modules/esm/loader:431:26)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:350:22)
    at new ModuleJob (node:internal/modules/esm/module_job:66:26)
    at ESMLoader.#createModuleJob (node:internal/modules/esm/loader:369:17)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:328:34)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:82:21)
    at async Promise.all (index 3)
    at link (node:internal/modules/esm/module_job:87:9)

So I cannot import a JSON file through the .pnp.loader.mjs.

I run the script with NO ISSUES (so JSON correctly imported) deleting lines 128:130 of .pnp.loader.mjs but every time I will run yarn install I will need to remove these lines again.

case `.json`: {
    throw new Error(`Unknown file extension ".json" for ${filepath}`);
}

@samuelexferri
Copy link

My temporary solution to update the lines 128:130 of .pnp.loader.mjs after a yarn install is to use this plugin (https://github.com/GravitywellUK/yarn-plugin-postinstall) in order to execute this command postinstall:

sed -i 's/case `.json`: {/case `.MY_YARN_PNP_LOADER_TEMP_FIX`: {/' .pnp.loader.mjs

My full .yarnrc.yml file:

nodeLinker: pnp

packageExtensions:
  chalk@*:
    dependencies:
      "#ansi-styles": "npm:ansi-styles@*"
      "#supports-color": "npm:supports-color@*"

plugins:
  - path: .yarn/plugins/@yarnpkg/plugin-typescript.cjs
    spec: "@yarnpkg/plugin-typescript"
  - path: .yarn/plugins/@yarnpkg/plugin-interactive-tools.cjs
    spec: "@yarnpkg/plugin-interactive-tools"
  - path: .yarn/plugins/@yarnpkg/plugin-postinstall.cjs
    spec: "https://raw.githubusercontent.com/gravitywelluk/yarn-plugin-postinstall/master/bundles/%40yarnpkg/plugin-postinstall.js"

# Fix for Yarn PNP Loader (Usign User's yarn-plugin-postinstall)
postinstall: "sed -i 's/case `.json`: {/case `.MY_YARN_PNP_LOADER_TEMP_FIX`: {/' .pnp.loader.mjs"

pnpEnableEsmLoader: true

yarnPath: .yarn/releases/yarn-3.2.2.cjs

@espoal
Copy link
Author

espoal commented Jul 29, 2022

thanks @samuelexferri really helpful.

I noticed than it's not yet fixed in yarn v4, I hope @merceyz can dedicate a little bit of love to this before release.

Otherwise could you point me in the direction to create a PR to fix this? Maybe we can start to mark this issue as confirmed?

@samuelexferri
Copy link

My temporary fix is just a work around. We must investigate why lines 128:130 of .pnp.loader.mjs are needed.
Looking at lines 50:55 of https://github.com/yarnpkg/berry/blob/b0511b9b332c4450927cd3a58da16dc6a5c3da46/packages/yarnpkg-pnp/sources/esm-loader/loaderUtils.ts there is a TODO.

 case `.json`: {
      // TODO: Enable if --experimental-json-modules is present
      // Waiting on https://github.com/nodejs/node/issues/36935
      throw new Error(
        `Unknown file extension ".json" for ${filepath}`,
      );
    }

We need to know the NODE_OPTIONS and allow the JSON import if there is the --experimental-json-modules flag.

@espoal
Copy link
Author

espoal commented Jul 29, 2022

amazing @samuelexferri you basically served me a solution.

@merceyz do you really think we should wait for nodejs/node#36935 or maybe you could accept a hot patch for now?

I could maybe pair with Samuele and provide a PR.

@evokeego
Copy link

evokeego commented Aug 9, 2022

Also hit this problem. Based on the info here I found that if you just replace that line in .pnp.loader.mjs to return 'commonjs' everything works fine. i.e.

-       throw new Error(`Unknown file extension ".json" for ${filepath}`);
+       return 'commonjs';

I didn't want to patch my .pnp.loader.mjs tho and since I'm currently using a custom loader that wraps .pnp.loader.mjs and tweaks the resolve step (that way I don't have to include .js at the end of all my local file imports), I just added a '.json' extension check in the load step and manually loaded the json myself which works great!

@merceyz
Copy link
Member

merceyz commented Aug 21, 2022

@merceyz do you really think we should wait for nodejs/node#36935 or maybe you could accept a hot patch for now?

Since Node.js unflagged JSON modules we no longer have to wait for that issue, I've opened #4786 which adds support for JSON modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working esm upholded Real issues without formal reproduction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants