diff --git a/.changeset/little-cherries-compete.md b/.changeset/little-cherries-compete.md new file mode 100644 index 000000000..063077eba --- /dev/null +++ b/.changeset/little-cherries-compete.md @@ -0,0 +1,5 @@ +--- +@swc/plugin-experimental-feature-flags: minor +--- + +feat(swc_feature_flags): Support indirect destructuring and property access patterns diff --git a/crates/swc_feature_flags/README.md b/crates/swc_feature_flags/README.md index 03747ffca..13de4942d 100644 --- a/crates/swc_feature_flags/README.md +++ b/crates/swc_feature_flags/README.md @@ -11,9 +11,8 @@ This library enables powerful feature flag management with aggressive dead code ## Features -- ✅ **Destructuring support**: `const { featureA, featureB } = useExperimentalFlags()` +- ✅ **Multiple usage patterns**: Direct destructuring, indirect destructuring, and property access - ✅ **Customizable function names**: Not hardcoded to specific function names -- ✅ **Selective processing**: Exclude specific flags from transformation - ✅ **Scope-safe**: Uses SWC's `Id` system to handle variable shadowing correctly - ✅ **Dead code elimination**: Removes unreachable code branches - ✅ **Statistics tracking**: Reports bytes removed and branches eliminated @@ -57,6 +56,43 @@ The plugin: 3. Replaces flag identifiers with `__SWC_FLAGS__.flagName` markers 4. Removes import statements and hook calls +### Supported Usage Patterns + +The build-time plugin supports multiple ways of accessing feature flags: + +#### Pattern 1: Direct Destructuring +```javascript +import { useExperimentalFlags } from '@their/library'; + +const { featureA, featureB } = useExperimentalFlags(); +if (featureA) { + // Transformed to: if (__SWC_FLAGS__.featureA) +} +``` + +#### Pattern 2: Indirect Destructuring +```javascript +import { useExperimentalFlags } from '@their/library'; + +const flags = useExperimentalFlags(); +const { featureA } = flags; +if (featureA) { + // Transformed to: if (__SWC_FLAGS__.featureA) +} +``` + +#### Pattern 3: Property Access +```javascript +import { useExperimentalFlags } from '@their/library'; + +const flags = useExperimentalFlags(); +if (flags.featureA) { + // Transformed to: if (__SWC_FLAGS__.featureA) +} +``` + +All three patterns are transformed identically and can be mixed in the same file. + ### Phase 2: Runtime Transformation The runtime transformer substitutes flag values and eliminates dead code: @@ -127,7 +163,6 @@ libraries.insert( let build_config = BuildTimeConfig { libraries, - exclude_flags: vec![], marker_object: "__SWC_FLAGS__".to_string(), }; @@ -171,7 +206,6 @@ program = program.apply(runtime_pass(runtime_config)); "functions": ["useFeatures"] } }, - "excludeFlags": ["quickToggle"], "markerObject": "__SWC_FLAGS__" }] ] @@ -189,9 +223,6 @@ interface BuildTimeConfig { /** Library configurations: library name -> config */ libraries: Record; - /** Flags to exclude from build-time marking */ - excludeFlags?: string[]; - /** Global object name for markers (default: "__SWC_FLAGS__") */ markerObject?: string; } diff --git a/crates/swc_feature_flags/src/build_time.rs b/crates/swc_feature_flags/src/build_time.rs index 4350b649e..567999ede 100644 --- a/crates/swc_feature_flags/src/build_time.rs +++ b/crates/swc_feature_flags/src/build_time.rs @@ -7,12 +7,20 @@ use swc_ecma_visit::{noop_visit_mut_type, VisitMut, VisitMutWith}; use crate::config::BuildTimeConfig; +/// Information about a flag object variable (e.g., `const flags = useFlags()`) +struct FlagObjectInfo { + span_lo: u32, // For removal tracking +} + /// Build-time transformer that replaces feature flag identifiers with /// __SWC_FLAGS__ markers pub struct BuildTimeTransform { config: BuildTimeConfig, /// Map of flag identifier Id -> flag name flag_map: HashMap, + /// Map of flag object variable Id -> info (for tracking `const flags = + /// useFlags()`) + flag_object_map: HashMap, /// Import sources to remove (library names) imports_to_remove: HashSet, /// Call expressions to remove (span-based tracking) @@ -33,6 +41,7 @@ impl BuildTimeTransform { Self { config, flag_map: HashMap::new(), + flag_object_map: HashMap::new(), imports_to_remove, declarators_to_remove: HashSet::new(), } @@ -53,62 +62,103 @@ impl BuildTimeTransform { false } + /// Extract flags from an object pattern and add them to flag_map + /// Returns true if any flags were extracted + fn extract_flags_from_object_pattern(&mut self, obj_pat: &ObjectPat) -> bool { + let mut extracted_any = false; + + for prop in &obj_pat.props { + if let ObjectPatProp::KeyValue(kv) = prop { + // Extract the flag name from the key + let flag_name = match &kv.key { + PropName::Ident(ident_name) => ident_name.sym.as_ref().to_string(), + PropName::Str(str_name) => { + // Convert Wtf8Atom to String + str_name + .value + .as_str() + .map(|s| s.to_string()) + .unwrap_or_else(|| { + // If not valid UTF-8, use lossy conversion + str_name.value.to_atom_lossy().to_string() + }) + } + _ => continue, + }; + + // Extract the local binding identifier + if let Pat::Ident(binding_ident) = &*kv.value { + let flag_id = binding_ident.id.to_id(); + self.flag_map.insert(flag_id, flag_name); + extracted_any = true; + } + } else if let ObjectPatProp::Assign(assign_prop) = prop { + // Shorthand: { flagA } = useFlags() + let flag_name = assign_prop.key.sym.to_string(); + + let flag_id = assign_prop.key.to_id(); + self.flag_map.insert(flag_id, flag_name); + extracted_any = true; + } + } + + extracted_any + } + /// Analyze a variable declarator to detect flag destructuring fn analyze_declarator(&mut self, declarator: &VarDeclarator) { - // Look for pattern: const { flagA, flagB } = useFlags() + // Pattern 1: const { flagA, flagB } = useFlags() if let Some(init) = &declarator.init { if let Expr::Call(call_expr) = &**init { if self.is_flag_function_call(&call_expr.callee) { - // This is a flag function call, extract flag names from pattern - if let Pat::Object(obj_pat) = &declarator.name { - for prop in &obj_pat.props { - if let ObjectPatProp::KeyValue(kv) = prop { - // Extract the flag name from the key - let flag_name = match &kv.key { - PropName::Ident(ident_name) => { - ident_name.sym.as_ref().to_string() - } - PropName::Str(str_name) => { - // Convert Wtf8Atom to String - str_name - .value - .as_str() - .map(|s| s.to_string()) - .unwrap_or_else(|| { - // If not valid UTF-8, use lossy conversion - str_name.value.to_atom_lossy().to_string() - }) - } - _ => continue, - }; - - // Skip if excluded - if self.config.exclude_flags.contains(&flag_name) { - continue; - } - - // Extract the local binding identifier - if let Pat::Ident(binding_ident) = &*kv.value { - let flag_id = binding_ident.id.to_id(); - self.flag_map.insert(flag_id, flag_name); - } - } else if let ObjectPatProp::Assign(assign_prop) = prop { - // Shorthand: { flagA } = useFlags() - let flag_name = assign_prop.key.sym.to_string(); - - // Skip if excluded - if self.config.exclude_flags.contains(&flag_name) { - continue; - } - - let flag_id = assign_prop.key.to_id(); - self.flag_map.insert(flag_id, flag_name); + match &declarator.name { + // Direct destructuring from flag function call + Pat::Object(obj_pat) => { + let extracted = self.extract_flags_from_object_pattern(obj_pat); + // Only remove if we actually extracted flags + if extracted { + self.declarators_to_remove.insert(declarator.span.lo.0); } } + // Pattern 2: const flags = useFlags() + Pat::Ident(ident) => { + let flag_id = ident.id.to_id(); + self.flag_object_map.insert( + flag_id, + FlagObjectInfo { + span_lo: declarator.span.lo.0, + }, + ); + // Don't remove yet - we'll remove only if flags are + // used + } + _ => {} } + return; + } + } + } + + // Pattern 3: const { flagA } = flags (indirect destructuring) + if let Pat::Object(obj_pat) = &declarator.name { + if let Some(init) = &declarator.init { + if let Expr::Ident(ident) = &**init { + let source_id = ident.to_id(); + // Get span_lo before calling extract method to avoid borrow checker issues + let source_span_lo = self + .flag_object_map + .get(&source_id) + .map(|info| info.span_lo); - // Mark this declarator for removal - self.declarators_to_remove.insert(declarator.span.lo.0); + if let Some(source_span) = source_span_lo { + let extracted = self.extract_flags_from_object_pattern(obj_pat); + // Only remove if we actually extracted flags + if extracted { + // Remove both the destructuring declaration and the source flag object + self.declarators_to_remove.insert(declarator.span.lo.0); + self.declarators_to_remove.insert(source_span); + } + } } } } @@ -191,6 +241,24 @@ impl VisitMut for BuildTimeTransform { *expr = self.create_flag_member_expr(flag_name); } } + + // Handle member expressions: flags.featureA → __SWC_FLAGS__.featureA + if let Expr::Member(member_expr) = expr { + if let Expr::Ident(obj_ident) = &*member_expr.obj { + let obj_id = obj_ident.to_id(); + + if let Some(info) = self.flag_object_map.get(&obj_id) { + if let MemberProp::Ident(prop_ident) = &member_expr.prop { + let flag_name = prop_ident.sym.to_string(); + + // Mark the flag object declaration for removal since we're transforming + // this usage + self.declarators_to_remove.insert(info.span_lo); + *expr = self.create_flag_member_expr(&flag_name); + } + } + } + } } } diff --git a/crates/swc_feature_flags/src/config.rs b/crates/swc_feature_flags/src/config.rs index 1aa9ab856..8ee265691 100644 --- a/crates/swc_feature_flags/src/config.rs +++ b/crates/swc_feature_flags/src/config.rs @@ -32,10 +32,6 @@ pub struct FeatureFlagsConfig { #[serde(default)] pub libraries: HashMap, - /// Flags to exclude from processing - #[serde(default)] - pub exclude_flags: Vec, - /// Global object name for markers (default: "__SWC_FLAGS__") /// Only used in mark mode #[serde(default = "default_marker_object")] @@ -59,11 +55,6 @@ pub struct BuildTimeConfig { /// Library configurations: library name -> config pub libraries: HashMap, - /// Flags to exclude from build-time marking (one-liners that don't need - /// DCE) - #[serde(default)] - pub exclude_flags: Vec, - /// Global object name for markers (default: "__SWC_FLAGS__") #[serde(default = "default_marker_object")] pub marker_object: String, @@ -110,7 +101,6 @@ impl Default for BuildTimeConfig { fn default() -> Self { Self { libraries: HashMap::new(), - exclude_flags: Vec::new(), marker_object: default_marker_object(), } } @@ -132,7 +122,6 @@ impl Default for FeatureFlagsConfig { Self { mode: TransformMode::default(), libraries: HashMap::new(), - exclude_flags: Vec::new(), marker_object: default_marker_object(), flag_values: HashMap::new(), collect_stats: true, diff --git a/crates/swc_feature_flags/src/lib.rs b/crates/swc_feature_flags/src/lib.rs index 52c5d36c4..68b2c09e2 100644 --- a/crates/swc_feature_flags/src/lib.rs +++ b/crates/swc_feature_flags/src/lib.rs @@ -21,7 +21,6 @@ //! //! let build_config = BuildTimeConfig { //! libraries, -//! exclude_flags: vec![], //! marker_object: "__SWC_FLAGS__".to_string(), //! }; //! @@ -77,7 +76,6 @@ use swc_ecma_visit::visit_mut_pass; /// functions: vec!["useExperimentalFlags".to_string()], /// }), /// ]), -/// exclude_flags: vec![], /// marker_object: "__SWC_FLAGS__".to_string(), /// }; /// @@ -143,7 +141,6 @@ pub fn runtime_pass(config: RuntimeConfig) -> impl Pass { /// functions: vec!["useExperimentalFlags".to_string()], /// }), /// ]), -/// exclude_flags: vec![], /// marker_object: "__SWC_FLAGS__".to_string(), /// flag_values: HashMap::new(), // Not used in mark mode /// collect_stats: false, @@ -155,7 +152,6 @@ pub fn runtime_pass(config: RuntimeConfig) -> impl Pass { /// let shake_config = FeatureFlagsConfig { /// mode: TransformMode::Shake, /// libraries: HashMap::new(), // Not used in shake mode -/// exclude_flags: vec![], /// marker_object: "__SWC_FLAGS__".to_string(), /// flag_values: HashMap::from([ /// ("featureA".to_string(), true), @@ -172,7 +168,6 @@ pub fn feature_flags_pass(config: FeatureFlagsConfig) -> Box { // Phase 1: Mark flags with __SWC_FLAGS__ markers let build_config = BuildTimeConfig { libraries: config.libraries, - exclude_flags: config.exclude_flags, marker_object: config.marker_object, }; Box::new(visit_mut_pass(BuildTimeTransform::new(build_config))) diff --git a/crates/swc_feature_flags/tests/fixture.rs b/crates/swc_feature_flags/tests/fixture.rs index 4f707a7a2..8c8819f45 100644 --- a/crates/swc_feature_flags/tests/fixture.rs +++ b/crates/swc_feature_flags/tests/fixture.rs @@ -29,7 +29,6 @@ fn build_time_fixture(input: PathBuf) { let config = BuildTimeConfig { libraries, - exclude_flags: vec![], marker_object: "__SWC_FLAGS__".to_string(), }; diff --git a/crates/swc_feature_flags/tests/fixture/build-time/indirect-destructuring/input.js b/crates/swc_feature_flags/tests/fixture/build-time/indirect-destructuring/input.js new file mode 100644 index 000000000..d097e9f32 --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/indirect-destructuring/input.js @@ -0,0 +1,10 @@ +import { useExperimentalFlags } from '@their/library'; + +function App() { + const flags = useExperimentalFlags(); + const { featureA } = flags; + + if (featureA) { + console.log('Feature A enabled'); + } +} diff --git a/crates/swc_feature_flags/tests/fixture/build-time/indirect-destructuring/output.js b/crates/swc_feature_flags/tests/fixture/build-time/indirect-destructuring/output.js new file mode 100644 index 000000000..81f418810 --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/indirect-destructuring/output.js @@ -0,0 +1,5 @@ +function App() { + if (__SWC_FLAGS__.featureA) { + console.log('Feature A enabled'); + } +} diff --git a/crates/swc_feature_flags/tests/fixture/build-time/mixed-patterns/input.js b/crates/swc_feature_flags/tests/fixture/build-time/mixed-patterns/input.js new file mode 100644 index 000000000..1b697af8a --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/mixed-patterns/input.js @@ -0,0 +1,12 @@ +import { useExperimentalFlags } from '@their/library'; + +function App() { + const { featureA } = useExperimentalFlags(); + const flags = useExperimentalFlags(); + const { featureB } = flags; + const useC = flags.featureC; + + if (featureA && featureB && useC && flags.featureD) { + console.log('All enabled'); + } +} diff --git a/crates/swc_feature_flags/tests/fixture/build-time/mixed-patterns/output.js b/crates/swc_feature_flags/tests/fixture/build-time/mixed-patterns/output.js new file mode 100644 index 000000000..b016ef11c --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/mixed-patterns/output.js @@ -0,0 +1,6 @@ +function App() { + const useC = __SWC_FLAGS__.featureC; + if (__SWC_FLAGS__.featureA && __SWC_FLAGS__.featureB && useC && __SWC_FLAGS__.featureD) { + console.log('All enabled'); + } +} diff --git a/crates/swc_feature_flags/tests/fixture/build-time/property-access/input.js b/crates/swc_feature_flags/tests/fixture/build-time/property-access/input.js new file mode 100644 index 000000000..9330bd169 --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/property-access/input.js @@ -0,0 +1,11 @@ +import { useExperimentalFlags } from '@their/library'; + +function App() { + const flags = useExperimentalFlags(); + + if (flags.featureA) { + console.log('Feature A enabled'); + } + + return flags.featureB ? 'Beta' : 'Stable'; +} diff --git a/crates/swc_feature_flags/tests/fixture/build-time/property-access/output.js b/crates/swc_feature_flags/tests/fixture/build-time/property-access/output.js new file mode 100644 index 000000000..7a7eb1578 --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/property-access/output.js @@ -0,0 +1,6 @@ +function App() { + if (__SWC_FLAGS__.featureA) { + console.log('Feature A enabled'); + } + return __SWC_FLAGS__.featureB ? 'Beta' : 'Stable'; +} diff --git a/crates/swc_feature_flags/tests/fixture/build-time/scope-safety-object/input.js b/crates/swc_feature_flags/tests/fixture/build-time/scope-safety-object/input.js new file mode 100644 index 000000000..72882003b --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/scope-safety-object/input.js @@ -0,0 +1,14 @@ +import { useExperimentalFlags } from '@their/library'; + +function App() { + const flags = useExperimentalFlags(); + + if (flags.featureA) { + console.log('Outer'); + + const flags = { featureA: false }; + if (flags.featureA) { + console.log('Inner - should use local flags'); + } + } +} diff --git a/crates/swc_feature_flags/tests/fixture/build-time/scope-safety-object/output.js b/crates/swc_feature_flags/tests/fixture/build-time/scope-safety-object/output.js new file mode 100644 index 000000000..23a8c79aa --- /dev/null +++ b/crates/swc_feature_flags/tests/fixture/build-time/scope-safety-object/output.js @@ -0,0 +1,11 @@ +function App() { + if (__SWC_FLAGS__.featureA) { + console.log('Outer'); + const flags = { + featureA: false + }; + if (flags.featureA) { + console.log('Inner - should use local flags'); + } + } +} diff --git a/packages/feature-flags/__tests__/__snapshots__/wasm.test.ts.snap b/packages/feature-flags/__tests__/__snapshots__/wasm.test.ts.snap index 698029bfb..02d685f77 100644 --- a/packages/feature-flags/__tests__/__snapshots__/wasm.test.ts.snap +++ b/packages/feature-flags/__tests__/__snapshots__/wasm.test.ts.snap @@ -7,18 +7,6 @@ exports[`Configuration defaults > Should default to mark mode when mode is not s " `; -exports[`Edge cases > Should handle exclude flags in mark mode 1`] = ` -"function App() { - if (__SWC_FLAGS__.includedFlag) { - console.log('Included'); - } - if (excludedFlag) { - console.log('Excluded'); - } -} -" -`; - exports[`Edge cases > Should handle nested scopes correctly in mark mode 1`] = ` "function App() { if (__SWC_FLAGS__.featureA) { diff --git a/packages/feature-flags/__tests__/wasm.test.ts b/packages/feature-flags/__tests__/wasm.test.ts index 5c2971490..f7353265f 100644 --- a/packages/feature-flags/__tests__/wasm.test.ts +++ b/packages/feature-flags/__tests__/wasm.test.ts @@ -207,39 +207,6 @@ function App() { }); describe("Edge cases", () => { - test("Should handle exclude flags in mark mode", async () => { - const input = `import { useExperimentalFlags } from '@their/library'; - -function App() { - const { includedFlag, excludedFlag } = useExperimentalFlags(); - - if (includedFlag) { - console.log('Included'); - } - - if (excludedFlag) { - console.log('Excluded'); - } -}`; - - const { code } = await transformCode(input, { - mode: "mark", - libraries: { - "@their/library": { - functions: ["useExperimentalFlags"], - }, - }, - excludeFlags: ["excludedFlag"], - }); - - expect(code).toMatchSnapshot(); - // includedFlag should be marked - expect(code).toContain("__SWC_FLAGS__.includedFlag"); - // excludedFlag should remain as variable - expect(code).toContain("excludedFlag"); - expect(code).not.toContain("__SWC_FLAGS__.excludedFlag"); - }); - test("Should handle nested scopes correctly in mark mode", async () => { const input = `import { useExperimentalFlags } from '@their/library'; diff --git a/packages/feature-flags/src/lib.rs b/packages/feature-flags/src/lib.rs index bdfd85f08..b881e6cd3 100644 --- a/packages/feature-flags/src/lib.rs +++ b/packages/feature-flags/src/lib.rs @@ -33,7 +33,6 @@ fn swc_plugin_feature_flags(mut program: Program, data: TransformPluginProgramMe // Phase 1: Mark flags with __SWC_FLAGS__ markers let build_config = BuildTimeConfig { libraries: config.libraries, - exclude_flags: config.exclude_flags, marker_object: config.marker_object, }; let mut transform = BuildTimeTransform::new(build_config);