-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(binding_core_wasm): Enable ecma_lints feature to support semantic error detection #11414
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
…c error detection This change adds the ecma_lints feature to the WASM binding, enabling critical lint rules such as duplicate variable declaration detection in the SWC playground. This aligns the playground's behavior with Babel and OXC playgrounds. The ecma_lints feature includes critical rules that detect: - Duplicate variable/const/let bindings - Duplicate exports - Const assignment violations - Duplicate function parameters Fixes #11041 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
|
|
|
|
🤖 This pull request has been linked to DevBird Task #3806 View the task details and manage the automated development workflow in DevBird. Learn more about DevBird here or the announcement blog post here. |
📋 DevBird Task PromptObjectiveFix the SWC playground so it emits semantic lint errors when duplicate variable declarations occur (e.g., Issue Reference
Root Causes Identified
Technical Requirements1. Enable ecma_lints Feature in WASM BindingFile: Add the swc_core = { path = "../../crates/swc_core", features = [
"ecma_ast_serde",
"ecma_codegen",
"binding_macro_wasm",
"ecma_transforms",
"ecma_visit",
"ecma_helpers_inline",
"ecma_lints", # ADD THIS LINE
] }2. Understand the Linting InfrastructureKey Files to Review:
How Linting Works:
3. Verify the Build ConfigurationThe WASM binding uses binding macros from
4. Testing StrategyManual Testing: const a = 1;
const a = 2;Expected output: Error message "the name Additional Test Cases: // Should error - duplicate const
const x = 1;
const x = 2;
// Should error - duplicate let
let y = 1;
let y = 2;
// Should NOT error - var allows redeclaration
var z = 1;
var z = 2;
// Should error - duplicate function parameters
function test(a, a) {}
// Should error - duplicate imports
import { foo } from 'bar';
import { foo } from 'baz';Running Tests:
Documentation & ReferencesOfficial SWC Documentation:
Related Code References:
GitHub Context:
SWC Playground Repository:
ScopeIn Scope:
Out of Scope:
Success Criteria
Implementation NotesRepository Context:
Build Commands: # Format code
cargo fmt --all
# Build WASM binding
cd bindings/binding_core_wasm
wasm-pack build
# Run lint tests
cargo test --package swc_ecma_lints
# Run error message tests
cargo test -p swc --test error_msgImportant Considerations:
This comment was automatically added by DevBird. You can disable this feature in DevBird Settings. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Code ReviewSummaryThis PR adds the ✅ Positives
🔍 Observations & Considerations1. Implicit IntegrationThe Question: How does the linting get activated in the WASM binding? Is it automatically integrated through the 2. Bundle Size ImpactAdding
Recommendation: Monitor the "Measure Binary Size" CI check to ensure the size increase is acceptable for the playground use case. 3. Missing Test CoverageThe PR description mentions the intended behavior but doesn't add tests to
Recommendation: Add a test case like: it('should detect duplicate variable declarations', () => {
const output = swc.transformSync('const a = 1; const a = 2;', {
jsc: {
parser: { syntax: 'ecmascript' }
}
});
expect(output.diagnostics.length).toBeGreaterThan(0);
expect(output.diagnostics[0]).toMatchObject({
message: expect.stringContaining('defined multiple times')
});
});4. Configuration Surface AreaThe main SWC crate has a
Recommendation: Document the configuration behavior or link to examples of how to use linting from the WASM binding. 5. Performance ImplicationsLinting adds an additional pass over the AST. For the playground use case this is fine, but:
🎯 Suggestions
🔒 Security & Best Practices
VerdictApprove with suggestions: The core change is sound and addresses a legitimate issue. The main improvement would be adding test coverage to verify the feature works as intended. The implicit integration through the transform pipeline should work, but explicit testing would provide confidence. Would love to see a test case added to confirm the linting behavior before merging. Otherwise, this is a good addition to the playground capabilities! 🚀 |
Binary Sizes
Commit: 0ef29fa |
CodSpeed Performance ReportMerging #11414 will not alter performanceComparing Summary
|
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 enables the ecma_lints feature in the WASM binding configuration to support semantic error detection in the SWC playground, bringing it to feature parity with Babel and OXC playgrounds.
Key Changes:
- Added
ecma_lintsfeature flag toswc_coredependency in WASM binding configuration
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This PR adds the
ecma_lintsfeature to the WASM binding configuration, enabling critical lint rules such as duplicate variable declaration detection in the SWC playground. This aligns the playground's behavior with Babel and OXC playgrounds.Changes
ecma_lintsto the feature list inbindings/binding_core_wasm/Cargo.tomlImpact
The
ecma_lintsfeature includes critical rules that detect:Testing
cargo checkcargo fmt --allto ensure code formattingTest Cases
After this change, the SWC playground should emit errors for code like:
Related Issues
Fixes #11041
🤖 Generated with Claude Code