Skip to content

fix(ext/node): emit DEP0198 warning for SHAKE digests without outputLength#32521

Merged
bartlomieju merged 4 commits intodenoland:mainfrom
bartlomieju:fix/crypto-shake-deprecation-warning
Mar 9, 2026
Merged

fix(ext/node): emit DEP0198 warning for SHAKE digests without outputLength#32521
bartlomieju merged 4 commits intodenoland:mainfrom
bartlomieju:fix/crypto-shake-deprecation-warning

Conversation

@bartlomieju
Copy link
Copy Markdown
Member

Summary

  • Emit DeprecationWarning (DEP0198) when creating SHAKE128/256 digests without an explicit options.outputLength, matching Node.js behavior with --pending-deprecation
  • Enables test-crypto-default-shake-lengths.js and test-crypto-default-shake-lengths-oneshot.js in node_compat

Test plan

  • cargo test --test node_compat test-crypto-default-shake-lengths — 2 tests pass

🤖 Generated with Claude Code

…ength

Emit a DeprecationWarning (DEP0198) when creating SHAKE128/256 digests
without an explicit `options.outputLength`, matching Node.js behavior
with `--pending-deprecation`.

Enables `test-crypto-default-shake-lengths.js` and
`test-crypto-default-shake-lengths-oneshot.js` in node_compat.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@kajukitli kajukitli left a comment

Choose a reason for hiding this comment

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

lgtm, straightforward change.

one minor nit: you're calling algorithm.toLowerCase() twice. could do:

const algoLower = algorithm.toLowerCase();
if (!isCopy && xofLen === undefined && (algoLower === "shake128" || algoLower === "shake256")) {

not a big deal since this only runs once per Hash construction, but cleaner.

Copy link
Copy Markdown
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM, with Kaju's nit

@bartlomieju bartlomieju enabled auto-merge (squash) March 9, 2026 15:42
@bartlomieju bartlomieju merged commit 98ca450 into denoland:main Mar 9, 2026
112 checks passed
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.

3 participants