-
Notifications
You must be signed in to change notification settings - Fork 1
chore: move electron to devDependencies and bump to 34.1.0, remove unstable loadingView test case #37
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
Conversation
* Initial plan * Move electron from dependencies to devDependencies and bump version to 34.1.0 Co-authored-by: z0gSh1u <30524126+z0gSh1u@users.noreply.github.com> * Remove package-lock.json as per project conventions Co-authored-by: z0gSh1u <30524126+z0gSh1u@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: z0gSh1u <30524126+z0gSh1u@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR moves the electron package from dependencies to devDependencies and bumps the package version from 34.0.0 to 34.1.0. The purpose is to prevent Electron from being bundled into consuming applications, which is a common issue when utility libraries incorrectly declare Electron as a regular dependency.
Changes:
- Version bumped from 34.0.0 to 34.1.0
electronpackage moved from dependencies to devDependencies
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@babel/eslint-parser": "^7.27.1", | ||
| "@playwright/test": "^1.52.0", | ||
| "@types/node": "^22.15.17", | ||
| "electron": "34", |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The electron package should be specified as a peerDependency rather than a devDependency. The library code requires electron at runtime (see lib/electron-windows.js line 4: require('electron')). This is a utility library for Electron apps, so electron should be provided by the consuming application. Consider adding it to peerDependencies instead and keeping it in devDependencies for development/testing purposes.
| "@babel/eslint-parser": "^7.27.1", | ||
| "@playwright/test": "^1.52.0", | ||
| "@types/node": "^22.15.17", | ||
| "electron": "34", |
Copilot
AI
Jan 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The electron version specification "34" is too loose and non-standard. It should use a proper semver range like "^34.0.0" or ">=34.0.0 <35.0.0" to ensure compatibility and follow npm best practices. The shorthand "34" is equivalent to ">=34.0.0 <35.0.0" but is less clear and not commonly used in modern npm packages.
| "electron": "34", | |
| "electron": ">=34.0.0 <35.0.0", |
* Initial plan * Remove unstable loadingView test case and exclude package-lock.json Co-authored-by: z0gSh1u <30524126+z0gSh1u@users.noreply.github.com> * Update .gitignore --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: z0gSh1u <30524126+z0gSh1u@users.noreply.github.com> Co-authored-by: ZHUO Xu <zx.cs@qq.com>
|
LGTM |
In this library,
electronis accidentally added as dependency, causing those Electron apps using this library to have an Electron copy packed into the artifact by default. This PR moves it into devDependencies, as other Electron utility libraries do.