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

Breaks under yarn >=3.0.0 #2332

Closed
leaumar opened this issue Oct 24, 2021 · 7 comments · Fixed by #2348
Closed

Breaks under yarn >=3.0.0 #2332

leaumar opened this issue Oct 24, 2021 · 7 comments · Fixed by #2348
Assignees

Comments

@leaumar
Copy link
Contributor

leaumar commented Oct 24, 2021

Is this a feature request or a bug?

Bug: crashes with a runtime error under a recent update of a popular package manager.

What is the current behavior?

This commit broke web-ext in my 6 addon projects. Updating yarn from 2.x to 3.0.0 and later causes the following error whenever invoking web-ext in any way (build, lint, run):

PS D:\Projects\copy-selected-links> yarn start
➤ YN0000: executing [start]: web-ext run
(node:7852) UnhandledPromiseRejectionWarning: TypeError: (0 , g.Parser) is not a function
    at R.cleanupProcessEnvConfigs (D:\Projects\copy-selected-links\.yarn\cache\web-ext-npm-6.5.0-1111bd61bd-fca1741464.zip\node_modules\web-ext\dist\web-ext.js:1:62943)
    at R.execute (D:\Projects\copy-selected-links\.yarn\cache\web-ext-npm-6.5.0-1111bd61bd-fca1741464.zip\node_modules\web-ext\dist\web-ext.js:1:63533)
    at Object.main (D:\Projects\copy-selected-links\.yarn\cache\web-ext-npm-6.5.0-1111bd61bd-fca1741464.zip\node_modules\web-ext\dist\web-ext.js:1:72644)
    at Object.<anonymous> (D:\Projects\copy-selected-links\.yarn\cache\web-ext-npm-6.5.0-1111bd61bd-fca1741464.zip\node_modules\web-ext\bin\web-ext:7:8)
    at Module._compile (internal/modules/cjs/loader.js:1085:14)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.external_module_.Module._load (D:\Projects\copy-selected-links\.pnp.cjs:24162:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:76:12)
    at internal/main/run_main_module.js:17:47

Rverting back to 2.4.3 solves the error.

Sole exception to this is yarn web-ext --version, which just works.

The folks over at yarn are adamant that issues like this are not on their table. 😕 I hope it will be received better here.

What is the expected or desired behavior?

Web-ext commands like run don't instantly fail with an error.

Version information (for bug reports)

  • Firefox version: n/a
  • Your OS and version: Win10
  • Paste the output of these commands:
node --version && npm --version && web-ext --version

14.18.1
6.14.15
6.4.0

Thanks

@rpl
Copy link
Member

rpl commented Oct 27, 2021

@leaumar it looks like the issue is actually between web-ext and yarn pnp module linker, because it seems to work correctly when nodeLinker .yarnrc.yml config is set to "node-modules" (which seems to be the default if the repository was using yarn 1 and then a v3 version is enabled through yarn set version 3.0.1).

I'm pretty sure that the code that is raising that exception is this one:

and my guess is that yarn pnp module linker may be confused by the fact that we are importing yargs's Parser class from "yargs/yargs" here:

I have never used yarn v3 and so this was the first time I actually learned of this yarn pnp feature, and so unfortunately I would need to dig much more to determine how we may achieve that in a way that doesn't behave differently in this yarn v3 mode.

Hopefully these details may help you to workaround the issue in the short run, until we may be able to take a deeper look and got a better picture of the reasons behind that unexpected behavior.

@leaumar
Copy link
Contributor Author

leaumar commented Oct 27, 2021

I've worked around it for now by downgrading to yarn 2.4.3. It kinda sounds to me like yargs doesn't export Parser correctly.

@leaumar
Copy link
Contributor Author

leaumar commented Nov 4, 2021

Ironically, there are other issues like eslint and ts-eslint with their recent updates (v8 and 5) requiring yarn >=3 now 😅 It's crazy how fickle the nodejs ecosystem is.

@leaumar
Copy link
Contributor Author

leaumar commented Nov 18, 2021

I just tried messing around with program.js and then debugging the faulty line during npm test to see if I could change anything about the imports or some such. The result is just weird behavior overall, not at all the usually predictable behavior I'm used to. It smells like hacks and the whole require/import... issue.

The only breadcrumb I can find in yargs itself is the dissimilarity between ./yargs and the other exports:

    ".": [
      {
        "import": "./index.mjs",
        "require": "./index.cjs"
      },
      "./index.cjs"
    ],
    "./helpers": {
      "import": "./helpers/helpers.mjs",
      "require": "./helpers/index.js"
    },
    "./yargs": [
      {
        "require": "./yargs"
      },
      "./yargs"
    ]

The rubber duck in me that isn't into npm packages and nodejs resolution hacks asks if the missing "import" on ./yargs could at all be related here, having to work through whatever magic yarn/pnp wraps around it. It couldn't be that simple, could it?

@leaumar
Copy link
Contributor Author

leaumar commented Nov 18, 2021

Changing import { Parser as yargsParser } from 'yargs/yargs'; to import { Parser as yargsParser } from 'yargs/helpers'; doesn't seem to break anything. I got the idea from this comment. Given the thing I noticed previously about the declarations in their package.json, I wanna try and see if it'll work this way under yarn.

@leaumar
Copy link
Contributor Author

leaumar commented Nov 18, 2021

Jesus christ, that fixes it...

All I did was open the web-ext dependency zip in the yarn cache, edit web-ext.js to change the one occurence of "yargs/yargs" to "yargs/helpers", and then re-ran the webext command in my repository, getting a successful run instead of the g.Parser is not a function.

image
image
image

(I'd just like to state to the open air that I'm getting fed up with all these "javascript as a language is evolving and stuck in buggy transition states on all fronts at once" issues)

@rpl
Copy link
Member

rpl commented Nov 19, 2021

❤️ @leaumar as someone that also got to investigate quite some "surprising issues with npm dependencies and co." over the last few years, I can't thank you enough for going deep into the yarn "rabbit hole" and identified a more than reasonable fix for the issue you were facing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants