feat: npm improvements - import maps, tree shaking, and dev/prod mode

[fcoury]

Summary

This PR implements the remaining npm improvement features for the Husk build system:

• Task 4.1: Import Maps Support - Configure CDN URLs for packages in husk.toml
• Task 4.2: Tree Shaking Hints - Add /*#__PURE__*/ annotations for better dead code elimination
• Task 4.3: Development vs Production Mode - Different optimizations based on dev flag

Changes

1. Import Maps Support

• Added import_map configuration to TargetConfig
• Import maps allow mapping package names to CDN URLs
• Example configuration:

  [targets.browser.import_map]
  react = "https://esm.sh/react@18"
  lodash = "https://cdn.skypack.dev/lodash"

2. Tree Shaking Hints

• Added tree_shaking flag to TargetConfig
• When enabled, adds /*#__PURE__*/ annotations to:
  • Constructor calls (e.g., new Date())
  • Object/JSON methods
  • Husk utility functions
• Helps bundlers like Webpack/Rollup eliminate unused code

3. Development vs Production Mode

• Added dev flag to TargetConfig
• Development mode features:
  • Debug comments for variables and functions
  • Runtime type assertions with console warnings
  • Tree shaking disabled for better debugging
• Production mode:
  • Clean output without debug artifacts
  • Tree shaking enabled
  • Optimized for performance

Testing

Added comprehensive integration tests in tests/integration/test_npm_features.sh that verify:

• Import maps configuration
• Tree shaking annotations
• Development mode debug features
• Production mode optimizations
• Combined feature interactions

All tests pass locally and in CI.

Example Usage

# husk.toml
[targets.browser]
tree_shaking = true
dev = false

[targets.browser.import_map]
react = "https://esm.sh/react@18"

[targets.browser-dev]
tree_shaking = false
dev = true

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

[fcoury]

@claude review this PR please

[claude]

Claude finished @fcoury's task —— View job

---

PR Review: NPM Improvements - Import Maps, Tree Shaking, and Dev/Prod Mode

I've completed a thorough review of this substantial PR implementing three key npm improvement features. Here's my comprehensive feedback:

✅ Strengths

1. Code Quality & Architecture

• Clean integration: New features are well-integrated into existing TargetConfig structure
• Consistent patterns: Follows established patterns for configuration parsing and target handling
• Proper defaults: Uses #[serde(default)] appropriately for new optional fields
• Good separation: Features are implemented in separate, focused methods

2. Feature Implementation

Import Maps (config.rs:114, transpiler.rs:1141-1162)

• ✅ Proper HashMap storage for package→URL mappings
• ✅ Integration with existing package resolution system
• ✅ Support for different import types (named, all, single)
• ✅ Target-specific configuration

Tree Shaking (transpiler.rs:1520-1569)

• ✅ Comprehensive pure function list (Math, Object, constructors, etc.)
• ✅ Smart detection of pure operations
• ✅ Correctly disabled in dev mode to preserve debugging
• ✅ Standard /*#__PURE__*/ annotations for bundler compatibility

Dev/Prod Mode (transpiler.rs:1571-1603)

• ✅ Debug comments for variables and functions
• ✅ Runtime type assertions with helpful warnings
• ✅ Conditional feature toggling (tree shaking disabled in dev)

3. Test Coverage

• ✅ Excellent unit test coverage: Separate test files for each feature
• ✅ Integration tests: Bash scripts testing real-world scenarios
• ✅ Edge cases covered: Dev/prod interactions, feature combinations
• ✅ Configuration testing: Various target configurations tested

⚠️ Areas for Improvement

1. Security Considerations

// src/transpiler.rs:1141-1162
fn generate_import_map_import(&self, mapped_specifier: &str, ...) -> Result<String> {
    // URL is used directly without validation
    Ok(format!("import * from '{}'", mapped_specifier))
}

Recommendation: Add URL validation for import map entries to prevent injection attacks or malformed URLs.

2. Repository Cleanup

The PR includes unnecessary files that should be cleaned up:

• src/lib.rs.bak - Backup file
• src/transpiler_target_modes_test.rs.bak - Backup file
• Root-level test files: test_cjs.husk, test_no_main.husk, test_module.rs
• Multiple test project directories in root

3. Performance Considerations

• Import map lookups: HashMap lookups are O(1), but consider caching for hot paths
• Pure annotation checking: String matching in should_add_pure_annotation could be optimized with a HashSet
• Dev mode overhead: Runtime type assertions add overhead - consider compile-time flags

4. Documentation

• The feature implementations are well-documented, but consider adding configuration examples to main README
• Import map security considerations should be documented

🔧 Minor Issues

Error Handling

The code properly uses Result types and avoids unsafe patterns like unwrap(). Good defensive programming practices.

Module Format Consistency

Good integration with existing module format detection in determine_module_format.

📊 Test Quality Assessment

Unit Tests: ⭐⭐⭐⭐⭐

• Comprehensive coverage of all features
• Good test organization with separate files
• Edge cases well covered

Integration Tests: ⭐⭐⭐⭐☆

• Real-world scenarios tested
• Bash scripts verify end-to-end functionality
• Could benefit from more error condition testing

🚀 Overall Assessment

This is a high-quality implementation of important npm ecosystem features. The code is well-structured, thoroughly tested, and follows Rust best practices. The three features work together coherently and provide significant value for JavaScript ecosystem integration.

Recommendation: Approve with minor cleanup

Required Actions Before Merge:

1. Remove backup files (.bak) and test artifacts from root directory
2. Add URL validation for import map entries
3. Consider documenting security considerations for import maps

Optional Improvements:

1. Optimize pure annotation checking with HashSet
2. Add configuration examples to documentation
3. Consider caching for import map lookups in hot paths

The implementation successfully addresses the npm improvement goals and significantly enhances Husk's JavaScript ecosystem compatibility.

---

[claude]

Claude finished @fcoury's task —— View job

---

PR Review: npm improvements - import maps, tree shaking, and dev/prod mode

Todo List:

• Read repository guidelines (CLAUDE.md)
• Review core configuration changes (config.rs)
• Review transpiler implementation (transpiler.rs)
• Review package resolver changes (package_resolver.rs)
• Examine test coverage and quality
• Check integration tests
• Provide comprehensive review feedback

---

Overall Assessment

This is a well-implemented PR that adds significant value to the Husk build system. The three new features (import maps, tree shaking hints, and dev/prod mode) are properly integrated with comprehensive test coverage. The implementation follows good software engineering practices with proper separation of concerns.

✅ Strengths

1. Excellent Test Coverage

• Dedicated test files for each feature (transpiler_import_maps_test.rs, transpiler_tree_shaking_test.rs, transpiler_dev_prod_test.rs)
• Comprehensive integration tests (test_npm_features.sh) that test end-to-end functionality
• Tests cover edge cases like disabled features, dev mode overrides, and different target platforms

2. Clean Configuration Integration

• New fields properly added to TargetConfig in config.rs:114-118
• Backward-compatible with #[serde(default)] annotations
• Intelligent package.json generation includes "sideEffects": false when tree shaking is enabled (config.rs:329-332)

3. Robust Implementation

• Import Maps: Proper resolution in visit_use() method with fallback to external deps and package resolution
• Tree Shaking: Comprehensive list of pure functions (transpiler.rs:1520-1540) with smart conditions (disabled in dev mode)
• Dev Mode: Helpful debug comments and runtime type assertions for better developer experience

4. Smart Feature Interactions

• Tree shaking automatically disabled in development mode (good UX decision)
• Import maps have proper precedence over external dependencies and package resolution
• Platform-specific handling (Node.js vs Browser vs Deno)

🔍 Areas for Improvement

1. Minor: Tree Shaking Logic transpiler.rs:1520

The pure annotation logic could be more conservative. Some functions marked as pure might have side effects in certain contexts:

// Consider being more selective about these:
"Math.random" => true,  // Actually has side effects (changes RNG state)
"JSON.parse" | "JSON.stringify" => true,  // Can throw exceptions

Suggestion: Consider moving Math.random and exception-throwing functions to a separate category or adding error handling context.

2. Enhancement: Import Map Validation

The current implementation doesn't validate import map URLs at configuration time. Consider adding validation in config.rs:validate():

// In config.rs validate() method
for (package, url) in &target_config.import_map {
    if !url.starts_with("http") && !url.starts_with("/") && !url.starts_with("./") {
        return Err(Error::new_config(format!(
            "Invalid import map URL for '{}': {}", package, url
        )));
    }
}

3. Documentation: Feature Interactions

While the implementation correctly handles feature interactions, the PR description could better document:

• Tree shaking is automatically disabled in dev mode
• Import maps take precedence over package resolution
• How external deps interact with import maps

4. Minor: Type Safety

The runtime type assertions in dev mode (transpiler.rs:1560-1574) only check basic types. Consider extending to check for null/undefined values which are common sources of runtime errors.

🚀 Implementation Highlights

Import Maps (transpiler.rs:1300-1310)

// Check if this package is mapped in the import map
if let Some(ref target_info) = self.target_info {
    if let Some(mapped_specifier) = target_info.import_map.get(package_name) {
        return self.generate_import_map_import(mapped_specifier, items);
    }
}

✅ Excellent: Clean precedence handling and proper fallback chain.

Tree Shaking Annotations (transpiler.rs:1545-1555)

fn should_add_pure_annotation(&self, function_name: &str) -> bool {
    if let Some(ref target_info) = self.target_info {
        if !target_info.tree_shaking || target_info.dev {
            return false;
        }
    }
    // ... pure function matching
}

✅ Smart: Properly disables tree shaking in dev mode for better debugging experience.

Development Mode Features (transpiler.rs:1580-1590)

fn add_debug_comment(&self, comment: &str) -> String {
    if let Some(ref target_info) = self.target_info {
        if target_info.dev {
            return format!("/* {} */", comment);
        }
    }
    String::new()
}

✅ Clean: Non-intrusive debug comments that help developers understand generated code.

🎯 Minor Suggestions

1. Consider adding a configuration option to control the verbosity of dev mode comments
2. Add validation for import map URLs at configuration parse time
3. Consider extending runtime type checking to include null/undefined checks
4. Document the precedence order of import resolution in the README or documentation

✅ Recommendation

APPROVE: This PR significantly enhances the Husk build system with well-thought-out features, excellent test coverage, and clean implementation. The minor suggestions above are enhancements rather than blocking issues.

The implementation demonstrates good understanding of JavaScript bundler best practices and provides valuable developer experience improvements through the dev/prod mode distinction.