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

Handlebars 4.5.0 results in an unmet peer dependency #1589

Closed
skinny85 opened this issue Oct 28, 2019 · 14 comments
Closed

Handlebars 4.5.0 results in an unmet peer dependency #1589

skinny85 opened this issue Oct 28, 2019 · 14 comments

Comments

@skinny85
Copy link

@skinny85 skinny85 commented Oct 28, 2019

Hey all,

I'm from the AWS CDK project. We are a transitive client of Handlebars through Jest. After the 4.5.0 release, our canaries started failing. It's because we check whether our dependency closure does not contain unmet peer dependencies with npm ls. After Handlebars 4.5.0 was released, this is what we see:

│   │ │ ├─┬ istanbul-reports@2.2.6 
│   │ │ │ └─┬ handlebars@4.5.0 
│   │ │ │   ├── UNMET PEER DEPENDENCY eslint@^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 
│   │ │ │   ├─┬ eslint-plugin-compat@3.3.0 
│   │ │ │   │ ├── @babel/runtime@7.6.3 deduped 
│   │ │ │   │ ├── ast-metadata-inferer@0.1.1 
│   │ │ │   │ ├─┬ browserslist@4.7.2 
│   │ │ │   │ │ ├── caniuse-lite@1.0.30001005 
│   │ │ │   │ │ ├── electron-to-chromium@1.3.296 
│   │ │ │   │ │ └─┬ node-releases@1.1.39 
│   │ │ │   │ │   └── semver@6.3.0 
│   │ │ │   │ ├── caniuse-db@1.0.30001005 
│   │ │ │   │ ├── lodash.memoize@4.1.2 deduped 
│   │ │ │   │ ├─┬ mdn-browser-compat-data@0.0.84 
│   │ │ │   │ │ └── extend@3.0.2 deduped 
│   │ │ │   │ └── semver@6.3.0 
│   │ │ │   ├── neo-async@2.6.1 
│   │ │ │   ├─┬ optimist@0.6.1 
│   │ │ │   │ ├── minimist@0.0.10 
│   │ │ │   │ └── wordwrap@0.0.3 deduped 
│   │ │ │   ├── source-map@0.6.1 deduped 
│   │ │ │   └─┬ uglify-js@3.6.4 
│   │ │ │     ├── commander@2.20.3 
│   │ │ │     └── source-map@0.6.1 deduped 

I believe the issue is this commit added a dependency on eslint-plugin-compat, which only has a devDependency on eslint (but a peer dependency on it!).

Would it be possible to release 4.5.1 that adds the dependency on "eslint": "^3.3.0"?

Thanks,
Adam

nwoltman added a commit to nwoltman/handlebars.js that referenced this issue Oct 29, 2019
@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

BTW, shouldn't eslint-plugin-compat be a devDependency? That's what the ReadMe on the package page say at least...

@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

Thanks so much for the quick reaction @nwoltman!

Do you have some idea when can 4.5.1 be released? I'm asking, as this is currently making our canaries fail, and I'm wondering whether I should wait for 4.5.1, or do something with the canaries temporarily to fix them.

@nwoltman
Copy link

@nwoltman nwoltman commented Oct 29, 2019

@skinny85 You could temporarily use my branch in your package.json until the fix is released.

{
  "dependencies": {
    "handlebars": "nwoltman/handlebars.js#patch-1"
  }
}
@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

Like I said, we're a transitive client of yours, so that's not really an option :(

@nwoltman
Copy link

@nwoltman nwoltman commented Oct 29, 2019

With Yarn you can use dependency resolutions. Unfortunately, npm doesn't have this functionality.

@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

Yep, we currently use NPM, although we plan to move to Yarn soon.

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 29, 2019

sorry about that. I wanted to add additional linting because just before my release, the saucelabs-tests failed again with internet-explorer. I am currently doing most of this stuff on my own and there is no review process for changes that I make myself.

Any idea how to check on such mistakes easily in the CI? Want to volunteer as a reviewer in the future?

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 29, 2019

I have commit the fix and I'm now waiting for the Travis-CI build to finish. Then I'll release 4.5.1

@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

Thanks @nknapp !

@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

Any idea how to check on such mistakes easily in the CI?

I can tell you what we do in our project. We have a test that first does an npm install, and then npm ls. The second command will fail if any peer dependencies are unmet.

Want to volunteer as a reviewer in the future?

Sure :). You've been super helpful with this issue, so I'm more than happy to return the favor. Feel free to tag me on any PRs you want feedback on.

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 29, 2019

Thanks.

Well, it is no failing peer dependency in the handlebars-project, because eslint is a dev-dependency. I probably have to create a new project in a sibling directory within the travis-container and run npm install ../handlebars.js or something like that.

@nknapp
Copy link
Collaborator

@nknapp nknapp commented Oct 29, 2019

Should be fixed in 4.5.1

@nknapp nknapp closed this Oct 29, 2019
@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

Well, it is no failing peer dependency in the handlebars-project, because eslint is a dev-dependency. I probably have to create a new project in a sibling directory within the travis-container and run npm install ../handlebars.js or something like that.

What I meant is you try it on a new package.json which declares a dependency on Handlebars. That would have caught this issue.

@skinny85
Copy link
Author

@skinny85 skinny85 commented Oct 29, 2019

And thanks again for the super quick fix!

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.