feat: Support both modes

[kdy1]

No description provided.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 0c5e49f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[claude]

Code Review - PR #581: feat: Support both modes

Thank you for this release! I've reviewed the changes and here's my feedback:

Summary

This PR bumps the version of @swc/plugin-experimental-feature-flags from 0.2.0 to 0.3.0 and merges the CHANGELOG into the README. The release includes support for both "mark" and "shake" transformation modes.

---

✅ Positive Observations

Code Quality:

• The plugin architecture is well-designed with clear separation between mark mode (BuildTimeTransform) and shake mode (RuntimeTransform)
• Good use of Rust's type system with the TransformMode enum and configuration structs
• Backward compatibility is maintained by falling back to BuildTimeConfig when the unified config fails to parse (packages/feature-flags/src/lib.rs:54-60)

Documentation:

• Excellent documentation in the README with clear examples for both modes
• TypeScript definitions are comprehensive and well-documented with JSDoc comments
• The two-phase workflow explanation helps users understand when to use each mode

Test Coverage:

• Good test coverage with tests for both mark and shake modes
• Tests cover edge cases like nested scopes, multiple libraries, and exclude flags
• Snapshot testing ensures transformations remain consistent

---

📝 Observations & Suggestions

1. Version Bump Semantics
The PR title says "feat: Support both modes" and this is marked as a minor version bump (0.2.0 → 0.3.0). However, looking at the code, both modes already exist in the codebase. This appears to be a documentation/release PR rather than a new feature implementation. Consider clarifying:

• Is this documenting existing functionality?
• Or was the shake mode just added in a previous commit?

2. CHANGELOG Duplication
The CHANGELOG content is now duplicated in both CHANGELOG.md and README.md (starting at packages/feature-flags/README.md:398). This could lead to maintenance issues. Consider:

• Using a build step to inject the CHANGELOG into the README, or
• Linking to the CHANGELOG.md file instead of duplicating content

3. Type Definitions Deprecation Notice
In packages/feature-flags/types.d.ts:146, BuildTimeConfig is marked as deprecated with the comment:

@deprecated Use FeatureFlagsConfig with mode: "shake" instead

This should say mode: "mark" not mode: "shake", since BuildTimeConfig corresponds to mark mode (marker generation), not shake mode.

4. Missing Validation
In shake mode, flagValues is required but there's no validation to ensure it's provided. Consider adding validation in packages/feature-flags/src/lib.rs:42-52 to provide a better error message if flagValues is missing in shake mode.

5. Test Coverage Gap
The tests don't verify error cases:

• What happens if shake mode is used without flagValues?
• What happens with invalid configuration?
• What happens if a flag referenced in code isn't in flagValues?

Consider adding negative test cases to ensure error handling is robust.

6. Performance Consideration
The plugin performs a full AST traversal for both modes. For large codebases, users running both phases might traverse the AST twice. This is acceptable given the two-phase design, but documenting the performance characteristics would be helpful.

---

🔒 Security

No security concerns identified. The plugin:

• Operates on AST transformations at build time
• Doesn't execute user code
• Doesn't introduce injection vulnerabilities

---

📋 Release Checklist

• ✅ Version number updated correctly
• ✅ CHANGELOG updated
• ✅ Tests passing (integration tests in progress)
• ⚠️ Consider fixing the type definition deprecation comment

---

Verdict

This is a solid release PR for a well-architected plugin. The main issues are minor documentation/maintenance concerns rather than functional problems. The code quality is high and the feature is well-tested.

Recommendation: Approve with minor suggestions for follow-up improvements.

---

Review performed by Claude Code