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

deps: fix webpack dependency #3697

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rtritto
Copy link

@rtritto rtritto commented Sep 14, 2024

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

  • In @electron-forge/plugin-webpack dependency move webpack to peer dependencies
  • In @electron-forge/template-webpack-typescript dependency add webpack dependency (transitive peer dependency of @electron-forge/plugin-webpack)

Fix #3695

@rtritto rtritto requested a review from a team as a code owner September 14, 2024 12:33
@rtritto
Copy link
Author

rtritto commented Sep 14, 2024

FYI @MarshallOfSound

Copy link
Member

@VerteDinde VerteDinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need webpack as a peerDependency here?

@rtritto
Copy link
Author

rtritto commented Sep 19, 2024

fork-ts-checker-webpack-plugin use webpack (peer dependency) imply that webpack is auto installed by package manager as transitive peer depenedncy.
All transitive peer dependencies must be used as dependencies or peer dependencies.
An article written by author of yarn package manager: Implicit Transitive Peer Dependencies

webpack can be added once because it's a peer dependency.

Copy link
Member

@MarshallOfSound MarshallOfSound left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't right, the plugin needs to have full control of the webpack version, not just set it correctly once when the template installs.

@rtritto
Copy link
Author

rtritto commented Sep 19, 2024

Maybe fork-ts-checker-webpack-plugin should use webpack as @electron-forge/plugin-webpack:

// packages/template/webpack-typescript/package.json and packages/template/webpack-typescript/tmpl/package.json
"devDependencies": {
  "webpack": "npm:@electron-forge/plugin-webpack@^7.4.0"
}

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

Successfully merging this pull request may close these issues.

[template-webpack-typescript] replace fork-ts-checker-webpack-plugin
3 participants