🦋 Changeset detected

Latest commit: 886afdf

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Action	Severity	Alert  (click "▶" to expand/collapse)
Warn		Critical CVE: npm cipher-base is missing type checks, leading to hash rewind and passing on crafted data CVE: GHSA-cpq7-6gpm-g9rc cipher-base is missing type checks, leading to hash rewind and passing on crafted data (CRITICAL) Affected versions: < 1.0.5 Patched version: 1.0.5 From: ? → npm/cipher-base@1.0.4 ℹ Read more on: This package | This alert | What is a critical CVE? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev. Suggestion: Remove or replace dependencies that include known critical CVEs. Consumers can use dependency overrides or npm audit fix --force to remove vulnerable dependencies. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/cipher-base@1.0.4. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.
Warn		Obfuscated code: npm buffer is 96.0% likely obfuscated Confidence: 0.96 Location: Package overview From: ? → npm/buffer@4.9.2 ℹ Read more on: This package | This alert | What is obfuscated code? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev. Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/buffer@4.9.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

Binary Sizes

Commit: 4d2cb38

Merging this PR will not alter performance

✅ 219 untouched benchmarks

_{Comparing kdy1/decorators-2023-11-babel-parity (886afdf) with main (1be052e)}

Code Review: feat(es/proposal) — Decorators 2023-11 Support

Impressive piece of work bringing full 2023-11 decorator parity with Babel. The approach of extending the existing decorator_impl.rs with version-gated paths is reasonable and keeps the codebase unified. Below are the findings, grouped by severity.

High Priority

1. is_unresolved_ident uses hard-coded context IDs 0 | 1 (decorator_impl.rs)

fn is_unresolved_ident(&self, ident: &Ident) -> bool {
    matches!(ident.ctxt.as_u32(), 0 | 1)
}

This is fragile — SyntaxContext values depend on how marks are allocated and could differ between runs or after refactors. The pass should store the actual unresolved_mark (or the SyntaxContext derived from it) and compare against that. False positives here would silently rewrite user code.

2. Implicit globals rewrite to globalThis.x runs unconditionally (decorator_impl.rs visit_mut_expr)
The rewrite of unresolved identifier assignments to globalThis.x = ... (and corresponding reads) appears to be for sloppy-mode compatibility with Babel fixtures. If this is only needed for test compatibility, it should not be in the production transform path — it changes semantics for all users. Consider gating this behind a test-only flag or removing it entirely if the Babel fixtures can be adjusted.

Medium Priority

3. prop.value.clone() where .take() would suffice (decorator_impl.rs, proto init injection)

if let Some(value) = prop.value.clone() {

The value is immediately overwritten on the next line, so the clone creates a wasted allocation. Using .take() would be more efficient and idiomatic.

4. Hard-coded helper name list in class_name_tdz.rs
The skip list of static-private helper names (_class_static_private_field_destructure, etc.) is matched by string. If helper names change or new ones are added, this list silently becomes stale. Consider a more structural approach (e.g., a helper attribute/tag) for robustness, or at minimum add a comment noting this coupling.

5. #[serde(deny_unknown_fields)] removed from BabelTestOptions (decorators.rs)
The original had deny_unknown_fields, but the new version silently ignores unknown option keys. While this was likely intentional for forward-compatibility with newer Babel options, it reduces test strictness — a typo in an option name would be silently swallowed rather than failing the test.

6. Duplicate utility code in _apply_decs_2311.js
The helper includes its own toPrimitive, toPropertyKey, checkInRHS, and setFunctionName implementations. If these exist as separate SWC helpers already, this is duplicated code that ships to users. This matches Babel's self-contained approach, so it may be intentional, but worth confirming.

Low Priority

7. visit_mut_class_members is ~540 lines long
This function handles every member kind (method, field, accessor, static, private) in one monolithic block. Extracting helpers per member kind would improve readability and testability.

8. collect::<Vec<_>>().into_iter() pattern

self.state.static_non_field_lhs.drain(..)
    .chain(...)
    .collect::<Vec<_>>()
    .into_iter()

The collect + into_iter creates a needless temporary allocation. The chain can be iterated directly.

9. static_block_mark declared but unused in test harness (decorators.rs)
Dead code — likely a leftover from development.

10. #[allow(unused)] at top of test file
This blanket suppression could mask real unused-code warnings. Consider narrowing it to specific items.

11. Obscure JS expression in _apply_decs_2311.js line 352

if ((kind & 8) == isStatic && !kindOnly == isField) {

!kindOnly == isField relies on loose-equality coercion between booleans and numbers. It works correctly but is hard to audit. Something like (kindOnly === 0) === !!isField would be clearer.

Positives

• Clean public API surface via decorator_2023_11.rs — minimal, well-structured entry point.
• The _apply_decs_2311.js helper is generally well-written: correct reverse-order application, proper addInitializer guarding, correct Symbol.metadata fallback, and no prototype pollution vectors (Object.create(null) for lookups).
• Good test coverage with extensive Babel fixture imports covering accessors, classes, methods, fields, ordering, and assumptions.
• The class_name_tdz.rs fix from sym comparison to to_id() comparison is correct and important — prevents false matches across scopes.
• Config wiring (config/mod.rs) is clean — replaces todo!() with the actual implementation.

Summary

The implementation is solid overall with good Babel parity. The two high-priority items (is_unresolved_ident magic numbers and the unconditional globalThis rewrite) should be addressed before merge as they affect correctness for production users. The medium-priority items are worth addressing but not blockers.

🤖 Generated with Claude Code