Skip to content

Conversation

@CalebBarnes
Copy link
Contributor

@CalebBarnes CalebBarnes commented Jun 5, 2025

Problem

Previously, extensions were being auto-installed on all accounts with the feature flag enabled, regardless of whether the required packages were present in package.json. This caused mass installations of extensions like Neon on accounts that didn't need them.

The core issues were:

  1. Incorrect filtering logic - Extensions were installed without properly checking if their required packages existed in project dependencies
  2. Wrong package.json location - Used cwd instead of buildDir for reading package.json
  3. Lack of comprehensive tests - No test coverage for the auto-install filtering logic

Solution

🔧 Core Bug Fixes

  • Fix extension filtering logic - Properly check if required packages exist in dependencies before installing
  • Use buildDir instead of cwd for package.json detection to respect Netlify's build configuration
  • Add comprehensive dependency matching - Extensions only install when their required packages are actually present

🧪 Comprehensive Test Suite

  • 5 test scenarios covering all edge cases:
    • Feature flag disabled/enabled
    • Missing package.json handling
    • Package detection logic (with/without required packages)
    • buildDir resolution with and without netlify.toml
  • 4 well-designed fixtures representing different project configurations
  • Pure test-only HTTP mocking for deterministic testing

🎯 Proper Extension Filtering

Now properly filters extensions to only install when:

  1. Extension is not already installed, AND
  2. At least one required package exists in project dependencies, AND
  3. Package.json is read from the correct buildDir location

Technical Details

Files Changed:

  • src/utils/extensions/auto-install-extensions.ts - Fixed filtering logic and updated to use buildDir
  • tests/extensions/ - Complete test suite with fixtures and HTTP mocking

Test Coverage:

  • Pure HTTP mocking - Global fetch mock intercepts external requests without implementation changes
  • buildDir resolution - Works with and without netlify.toml
  • Package detection - Correctly identifies required dependencies
  • Error handling - Graceful handling of missing package.json
  • Feature flag behavior - Respects enabled/disabled states

HTTP Mocking Strategy:

  • Test server handles internal API calls (site info, integrations, etc.)
  • Global fetch mock intercepts external extension API and installation requests

Impact

This fix prevents the mass installation bug that was causing extensions to be installed on accounts regardless of whether they had the required packages. The comprehensive test suite ensures this regression won't happen again and provides confidence in the auto-install logic across different project configurations.

Testing

npm test -- tests/extensions/tests.js

All tests pass and verify:

  • Correct extension filtering logic (only install when packages are present)
  • Proper package.json location resolution (buildDir vs cwd)
  • Complete HTTP request mocking without implementation pollution
  • Edge case handling (missing files, different configurations)

For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures
    we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
    something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures
    your code follows our style guide and passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

Previously, extensions were being auto-installed on all sites with the
feature flag enabled, regardless of whether the required packages were
present in package.json. This caused mass installations of extensions
like Neon on accounts that didn't need them.

Now properly filters extensions to only install when:
1. Extension is not already installed, AND
2. At least one required package exists in project dependencies

Fixes the logic in handleAutoInstallExtensions to check package.json
dependencies before attempting installation.
@CalebBarnes CalebBarnes requested a review from a team as a code owner June 5, 2025 01:17
Previously, auto-install extensions used `cwd` to look for package.json,
but this isn't always the correct location. The config system calculates
`buildDir` which represents the proper base directory where package.json
should be found (either `build.base` or `repositoryRoot`).

Changes:
- Pass `buildDir` instead of `cwd` to handleAutoInstallExtensions
- Update parameter and error logging to use buildDir
- Gracefully fail if no package.json found in buildDir
- Maintain existing package detection logic without new dependencies
- Use buildDir instead of cwd when reading package.json in auto-install extensions
- Add extensionApiBaseUrl parameter to fetchAutoInstallableExtensionsMeta for test mocking
- Update main config to use testOpts.host for extension API calls in test environment
- Add comprehensive test suite covering:
  - Feature flag disabled/enabled scenarios
  - Missing package.json graceful handling
  - Package detection logic (with/without required packages)
  - buildDir resolution with and without netlify.toml
- Mock auto-installable extensions API for deterministic testing

Fixes mass installation bug where extensions were installed on all sites
regardless of whether required packages were present in dependencies.
…ints

- Add global fetch mock to intercept external extension installation requests
- Mock installation endpoint responses without modifying implementation code
- Track installation requests for comprehensive test assertions
- Verify correct HTTP method, URL, headers, and request body
- Add unique mock response identifiers for verification
- Maintain clean separation between test and production code

This approach properly mocks external HTTP requests using test-only code,
ensuring no real network calls are made during testing while preserving
the original implementation behavior.
@CalebBarnes CalebBarnes changed the title fix: only auto-install extensions when required packages are present fix: auto-install extension filtering logic and add comprehensive tests Jun 5, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2025

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

- Revert implementation changes to fetchAutoInstallableExtensionsMeta and main.ts
- Extend global fetch mock to handle both installation and extension API endpoints
- Remove server-side mocks in favor of pure fetch interception
- Maintain clean separation between test and production code

This approach provides complete HTTP mocking without polluting the implementation
with test-specific logic, following proper testing best practices.
@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2025

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

@CalebBarnes CalebBarnes requested review from khendrikse and ndhoule June 5, 2025 02:59
@khendrikse khendrikse merged commit 0296018 into main Jun 5, 2025
87 of 89 checks passed
@khendrikse khendrikse deleted the fix/auto-install-extension-filter-logic branch June 5, 2025 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants