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

don't use deprecated buffer function #474

Open
ichub opened this issue Aug 29, 2023 · 7 comments
Open

don't use deprecated buffer function #474

ichub opened this issue Aug 29, 2023 · 7 comments
Labels

Comments

@ichub
Copy link
Contributor

ichub commented Aug 29, 2023

Screenshot 2023-08-29 at 4 26 13 PM
@robknight
Copy link
Member

This looks like a problem with the js-sha256 library, which is a dependency for the halo-nonce-pcd package. There's an unmerged PR for it: emn178/js-sha256#34

@robknight
Copy link
Member

The immediate cause of the warning which shows up when running yarn dev seems to be the use of new Buffer() in qr-image, which was last released 7 years ago. Getting upstream fixes for all uses of new Buffer() might be tricky, and moving to new dependencies which don't have this might introduce bigger problems.

There's an official guide to fixing code here, but it also points out that the use of the incorrect form is widespread and has even increased recently.

@ichub
Copy link
Contributor Author

ichub commented Sep 6, 2023

The immediate cause of the warning which shows up when running yarn dev seems to be the use of new Buffer() in qr-image, which was last released 7 years ago. Getting upstream fixes for all uses of new Buffer() might be tricky, and moving to new dependencies which don't have this might introduce bigger problems.

There's an official guide to fixing code here, but it also points out that the use of the incorrect form is widespread and has even increased recently.

Just curious but how were you able to tell that the qr-image package was the cause?

@robknight
Copy link
Member

robknight commented Sep 6, 2023

In apps/passport-server/package.json, add --trace-deprecation to the NODE_OPTIONS env var set during the dev task, and it outputs this:

passport-server:dev:     at showFlaggedDeprecation (node:buffer:193:11)
passport-server:dev:     at new Buffer (node:buffer:271:3)
passport-server:dev:     at ../../node_modules/qr-image/lib/png.js (/Users/robknight/Projects/zupass/packages/zk-eddsa-ticket-pcd/dist/index.js:9954:20)
passport-server:dev:     at __require (/Users/robknight/Projects/zupass/packages/zk-eddsa-ticket-pcd/dist/index.js:12:50)
passport-server:dev:     at ../../node_modules/qr-image/lib/qr.js (/Users/robknight/Projects/zupass/packages/zk-eddsa-ticket-pcd/dist/index.js:10262:15)
passport-server:dev:     at __require (/Users/robknight/Projects/zupass/packages/zk-eddsa-ticket-pcd/dist/index.js:12:50)
passport-server:dev:     at ../passport-ui/dist/index.js (/Users/robknight/Projects/zupass/packages/zk-eddsa-ticket-pcd/dist/index.js:10486:36)
passport-server:dev:     at __require (/Users/robknight/Projects/zupass/packages/zk-eddsa-ticket-pcd/dist/index.js:12:50)
passport-server:dev:     at Object.<anonymous> (/Users/robknight/Projects/zupass/packages/zk-eddsa-ticket-pcd/dist/index.js:10814:34)
passport-server:dev:     at Module._compile (node:internal/modules/cjs/loader:1233:14)

This particular instance would be fixed upstream by this PR, but since the last update was 5 years ago I would not be too hopeful of it being merged.

@ichub ichub added the bug label Sep 12, 2023
@robknight
Copy link
Member

This is not fixable except by changes to upstream packages, or by switching to different dependencies. In normal usage, node will ignore deprecation warnings that come from node_modules, but these warnings are occurring when code is being run after bundling, when it's not longer possible to distinguish our code from imported modules.

@ichub
Copy link
Contributor Author

ichub commented Sep 13, 2023

OK. let's leave it be for now.

@cha0sg0d
Copy link
Contributor

Was about to open this issue for qr-image and saw this discussion. Agree it's annoying but doesn't seem like any good fixes. +1 to leave it be for now

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

No branches or pull requests

3 participants