berry icon indicating copy to clipboard operation
berry copied to clipboard

[Bug?]: PnP loader cannot handle jsons

Open espoal opened this issue 4 years ago • 8 comments

Self-service

  • [x] 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 avatar Mar 18 '22 21:03 espoal

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

CarlosBalladares avatar Jul 04 '22 05:07 CarlosBalladares

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

CarlosBalladares avatar Jul 04 '22 06:07 CarlosBalladares

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 avatar Jul 27 '22 11:07 samuelexferri

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

samuelexferri avatar Jul 27 '22 14:07 samuelexferri

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?

espoal avatar Jul 29 '22 11:07 espoal

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.

samuelexferri avatar Jul 29 '22 11:07 samuelexferri

amazing @samuelexferri you basically served me a solution.

@merceyz do you really think we should wait for https://github.com/nodejs/node/issues/36935 or maybe you could accept a hot patch for now?

I could maybe pair with Samuele and provide a PR.

espoal avatar Jul 29 '22 11:07 espoal

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!

evokeego avatar Aug 09 '22 01:08 evokeego

@merceyz do you really think we should wait for https://github.com/nodejs/node/issues/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 https://github.com/yarnpkg/berry/pull/4786 which adds support for JSON modules.

merceyz avatar Aug 21 '22 18:08 merceyz