-
Notifications
You must be signed in to change notification settings - Fork 20
Update xlsx dependency to version 0.20.3 for improved security #1016
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
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 updates the xlsx library dependency from version 0.18.5 to 0.20.3, with the stated goal of improving security. However, instead of using the standard npm registry, it sources the package from a CDN (https://cdn.sheetjs.com).
Key Changes:
- Updated xlsx dependency from npm version ^0.18.5 to CDN-hosted version 0.20.3
- Removed transitive dependencies (adler-32, cfb, codepage, crc-32, ssf, wmf, word) as they're bundled in the new version
- Multiple peer dependency marking changes throughout package-lock.json
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| electron/package.json | Changes xlsx dependency source from npm registry to CDN URL for version 0.20.3 |
| electron/package-lock.json | Updates lock file with new xlsx version, removes bundled dependencies, and adjusts peer dependency markers |
Files not reviewed (1)
- electron/package-lock.json: Language not supported
e77e03b to
e9ee1f4
Compare
|
Not sure what copilot means, that version does not exist. I tested that using the following also compiles in our codebase diff --git a/electron/package.json b/electron/package.json
index 9b850629..0cc39b85 100644
--- a/electron/package.json
+++ b/electron/package.json
@@ -103,13 +103,13 @@
"react-virtualized": "^9.22.6",
"rehype-raw": "^7.0.0",
"remark-gfm": "^4.0.1",
+ "sheetjs": "^2.0.0",
"socket.io-client": "^4.8.1",
"socket.io-msgpack-parser": "^3.0.2",
"sonner": "^2.0.1",
"tailwind-merge": "^3.0.1",
"tailwindcss-animate": "^1.0.7",
"uplot": "^1.6.32",
- "xlsx": "^0.18.5",
"zod": "^3.24.2",
"zod-validation-error": "^3.4.0",
"zustand": "^5.0.8"I cannot make out, why two packages exist that seemingly both originate from the same group |
|
@TheBest6337 state on this? |
e9ee1f4 to
0dd2494
Compare
Sheet JS on NPM seems to be an placeholder lol (https://www.npmjs.com/package/sheetjs) |
Oshgnacknak
left a comment
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.
By thins point, I think I'd be easier to just export as csv. Excel registers itself as the default program for csvs anyway. Also one less dependency. If we need xlxs (not even odf will do), fix the hash and merge, if you ask me.
To fix the hash, you need to empty "" it in the electron.nix file and push once. CI will tell you the new hash. Just amend and force push you commit, then CI should pass.
smh, i dunno why they don't always print the new hash

No description provided.