Skip to content

Conversation

eduardoboucas
Copy link
Member

@eduardoboucas eduardoboucas commented Apr 3, 2024

Summary

Updates the logic used to determine the plugin version that should be picked when there is no pinned version for the site. Includes a feature flag that toggles between just logging whenever there's a mismatch between the legacy and new logic and actually applying the new version.

@eduardoboucas eduardoboucas requested review from a team as code owners April 3, 2024 11:55
Copy link
Contributor

github-actions bot commented Apr 3, 2024

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

@eduardoboucas eduardoboucas changed the title chore: add test for plugin compatibility check fix: update logic for plugin selection fallback Apr 3, 2024
buildDir: '/some/path',
pinnedVersion: '4',

test('if no pinned version is set, it matches the first version regardless of whether its requirements match the conditions (legacy behavior)', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff makes it a bit hard to see (sorry), but this is one of the new tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other reviewers - https://github.com/netlify/build/pull/5575/files?w=1 (hide whitespace changes in diff) makes this diff much easier to parse

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other reviewers - https://github.com/netlify/build/pull/5575/files?w=1 (hide whitespace changes in diff)

TIL, ty!

packageJson: { dependencies: { next: '14.0.0' } },
buildDir: '/some/path',
pinnedVersion: '4',
test('if no pinned version is set, it matches the first version whose requirements match the conditions', async () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diff makes it a bit hard to see (sorry), but this is one of the new tests.

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