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

Allowing acorn ^5 or ^6 as peer dependency breaks any module that relies on acorn 6 if 7 is present #103

Closed
iansan5653 opened this issue Nov 3, 2019 · 9 comments

Comments

@iansan5653
Copy link

iansan5653 commented Nov 3, 2019

Sorry for the confusing title - if anyone can improve it please do.

Problem

This module claims to support both acorn^6.0.0 and acorn^7.0.0:

"acorn": "^6.0.0 || ^7.0.0"

This may be correct in this particular module, but by allowing either one as a peer dependency, Node automatically pulls in the most recent one that is available. So if this plugin is being used in a project that still doesn't support acorn^7 but acorn^7 is included somewhere else in the dependency tree, the project will break because the versions conflict.

This is probably best explained with an example:

Example

I've uploaded a basic module at https://github.com/iansan5653/simple-acorn-jsx-example/ that is a minimal example of a module that uses this extension and still runs acorn^6. If you download this and run npm i and node index, the module will run just fine. As expected, npm installs the most recent version of acorn-jsx and acorn^6.3.0, and because acorn-jsx does support acorn^6, this works great.

However, if I require my example module in another project that uses acorn^7, this is where problems occur. Here's an example of such a project:

package.json:

  "dependencies": {
    "acorn": "^7.0.0",
    "simple-acorn-jsx-example": "github:iansan5653/simple-acorn-jsx-example"
  }
}

index.js:

const result = require("simple-acorn-jsx-example")
console.log(result);

Running node index here results in an error. This happens because acorn-jsx finds acorn^7 in its peers and pulls that in instead of using the version of acorn found inside simple-acorn-jsx-example.

Impact

A real-world impact of this is that any project which relies on Buble breaks if that project (or any of its dependencies) relies on acorn^7. By extension, any project that uses styleguideist breaks under the same circumstances, because styleguideist relies on Buble.

Reference: styleguidist/react-styleguidist#1321.

Solution

I'm not sure exactly what solution works best here. Ideally acorn-jsx should check to see which version of acorn the requiring package is using and then adjust accordingly, but that may not be feasible.

Maybe there should be two releases of this plugin - one that is compatible with acorn^4 and one with acorn^5. Maybe these could be two major versions of the same plugin? I'm not really sure.

Maybe there's a much more elegant solution I don't know about.

@RReverser
Copy link
Member

RReverser commented Nov 3, 2019

Hmm, you're saying that acorn-jsx "pulls in" a different version of acorn, but peerDependencies are not installed automatically by Node.js.

You need to add acorn^6 (or any other desired version) explicitly alongside acorn-jsx - it's not very clear from example above if that's what you're doing? (UPD: looks like you're in that Git repo)

@RReverser
Copy link
Member

Actually, I don't see a problem with your example. When I replicate same package.json and index.js, I do see an error, but it's coming from the acorn^6 inside simple-acorn-jsx-example dependencies, and not from the top-level one.

@iansan5653
Copy link
Author

iansan5653 commented Nov 3, 2019

Hmm maybe I'm wrong about the exact cause. I made the demo last night and it did what I expected so I figured I was right.

That said I'm not sure what else could be the cause. simple-acorn-jsx-example runs on its own just fine if it's not being used as a dependency. And it installs its own acorn^6 in its own node_modules if the requiring project installs acorn^7, as you can see in my example. Something is definitely conflicting and I don't think the error still happens if simple-acorn-jsx-example is modified to not depend on acorn-jsx (can't try it at the moment as I'm on mobile).

@RReverser
Copy link
Member

And it installs its own acorn^6 in its own node_modules if the requiring project installs acorn^7, as you can see in my example.

That's the expected behaviour, given that simple-acorn-jsx-example depends on acorn^6, no?

@RReverser
Copy link
Member

cc @mysticatea could this be similar to the issue you had and described earlier in acornjs/acorn#870 and #102?

@mysticatea
Copy link
Contributor

Yes, this looks the problem that acornjs/acorn#870 and #102 have solved. If you use acorn 7.1.0 or newer, this kind of broken will disappear.

If we want to solve this on acorn 6 as well, we need to backport acornjs/acorn#870.

@iansan5653
Copy link
Author

Backporting to 6 sounds like the ideal solution if that's feasible.

@huozhi
Copy link

huozhi commented Nov 9, 2019

encounter similar issue here:

having both rollup and buble as dependencies.
using rollup@1.25.2 > (deps on) > acorn@7.1.0
using buble@0.19.8, it requires acorn@6, which let npm install its own acorn@6 under buble folder.

then acorn-jsx parsing will break.

@iansan5653
Copy link
Author

Looks like this is resolved by acornjs/acorn#887 🎉

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

No branches or pull requests

4 participants