ensure .js extension on imports

[netanelgilad]

No description provided.

[claude]

Claude finished @netanelgilad's task —— View job

---

PR Review: ensure .js extension on imports

Summary

This PR adds ESLint configuration to enforce .js extensions on imports and sets up a GitHub Actions workflow for linting. The codebase already follows this pattern, so this change adds automated enforcement.

✅ Code Quality and Best Practices

Positive aspects:

• The ESLint rule import/extensions with ["error", "always", { ignorePackages: true }] is the correct configuration for ESM modules
• Using ESLint v9 with flat config format is modern and forward-looking
• The GitHub Actions workflow uses Node.js 20.x and proper caching
• Removed unused packageManager field from package.json

Suggestions:

• Consider adding npm run lint to your pre-commit hooks to catch issues earlier
• The ESLint config only targets src/**/*.ts files - consider if you need linting for other file types (e.g., .js files in scripts/)

✅ Potential Bugs or Issues

No bugs identified. The configuration is straightforward and the rule enforcement matches the existing code style.

✅ Performance Considerations

• GitHub Actions workflow uses npm ci which is faster than npm install for CI environments
• Node package caching is properly configured, which will speed up subsequent runs
• ESLint itself has minimal performance impact during development

✅ Security Concerns

No security issues identified. All dependencies are from trusted sources:

• @typescript-eslint/parser and eslint-plugin-import are well-maintained packages
• GitHub Actions uses official actions (actions/checkout@v4, actions/setup-node@v4)

⚠️ Test Coverage

Missing:

• No tests added to verify the ESLint configuration works as expected
• Consider adding a test file with intentionally missing .js extensions to ensure the linter catches these errors
• The lint job could be added as a required status check for PRs

Additional Observations

1. ESLint version upgrade: The PR upgrades ESLint from v8.54.0 to v9.39.2, which is a major version bump. This is good for staying current.

2. TypeScript ESLint: Added typescript-eslint v8.51.0 which provides the latest TypeScript support.

3. Workflow triggers: The workflow runs on pushes to main and PRs targeting main, which is appropriate for a linting workflow.

Recommendation

Approved - This is a good addition that enforces existing conventions and improves code quality through automation. The implementation is clean and follows best practices.

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.11-pr.64.98ee6d9

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.11-pr.64.98ee6d9"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.11-pr.64.98ee6d9"
  }
}

• 📦 Preview Package: @base44-preview/sdk@0.8.11-pr.64.98ee6d9
• 🔗 View this commit on GitHub

---

_{Preview published to npm registry — try new features instantly!}

[claude]

Claude finished @netanelgilad's task —— View job

---

PR Review: Ensure .js Extension on Imports

I've completed my review of this PR that adds ESLint configuration to enforce .js extensions on imports.

Review Checklist

• Gather context and check for CLAUDE.md
• Review ESLint configuration
• Review GitHub workflow configuration
• Review package.json changes
• Review TypeScript import changes
• Provide comprehensive feedback

Summary

This PR adds ESLint configuration to enforce .js extensions on all relative imports, which is a requirement for ES modules. The changes look good overall with only minor suggestions.

Code Quality ✅

ESLint Configuration (eslint.config.js)

The configuration is clean and focused:

• Correctly uses the modern flat config format
• Targets only TypeScript files in src/
• Uses @typescript-eslint/parser appropriately
• The import/extensions rule is configured to require extensions always except for packages

GitHub Workflow (.github/workflows/lint.yml)

Well-structured workflow:

• Runs on pushes to main and PRs
• Uses Node.js 20.x (good choice)
• Properly caches npm dependencies
• Simple and effective

Package.json Changes

• Added necessary ESLint dependencies
• Added "lint": "eslint src" script
• All versions are up-to-date

Potential Issues 🔍

1. ESLint Version: Using ESLint v9.39.2 with the new flat config is correct, but ensure all team members are aware of the config format change from .eslintrc to eslint.config.js.

2. TypeScript Compilation: The project uses "module": "esnext" in tsconfig.json, which is compatible with ES modules and .js extensions.

Performance Considerations ⚡

• The lint job in CI is lightweight and should add minimal overhead
• Running ESLint only on src/**/*.ts files keeps the scope focused

Security Concerns 🔒

No security issues identified. All dependencies are from trusted sources.

Test Coverage 🧪

The PR doesn't include tests, but that's appropriate since:

• This is a linting configuration change
• The CI workflow itself serves as validation
• All existing imports have been properly updated

Suggestions for Improvement 💡

1. Consider adding a pre-commit hook to run ESLint locally before commits:

   // In package.json
   "scripts": {
     "lint:fix": "eslint src --fix"
   }

2. Documentation: Consider adding a note in the README or contributing guidelines about the .js extension requirement for imports.

3. Migration completeness: I verified that all 15+ files in the src/ directory have been updated with .js extensions - great job on the thoroughness!

Conclusion

This is a well-executed PR that properly configures ESLint to enforce ES module best practices. The implementation is clean, focused, and follows modern JavaScript/TypeScript conventions. The addition of CI integration ensures consistent enforcement across the team.

Recommendation: Approve ✅