Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: now builds fail if the Next runtime is >= 4.0.0 && < 4.26.0 #4668

Conversation

nickytonline
Copy link
Contributor

@nickytonline nickytonline commented Nov 2, 2022

πŸŽ‰ Thanks for submitting a pull request! πŸŽ‰

Summary

I haven't tried this out yet and it needs tests.

Fixes #<replace_with_issue_number>


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)

@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

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

@nickytonline nickytonline force-pushed the nickytonline/fail-build-if-next-runtime-requires-update branch 2 times, most recently from e259dc0 to dee6701 Compare November 2, 2022 23:08
@nickytonline nickytonline force-pushed the nickytonline/fail-build-if-next-runtime-requires-update branch from dee6701 to 2266671 Compare November 2, 2022 23:09
if (outdatedPlugins.length === 0) {
return
}

throwIfOutdatedNextRuntime(outdatedPlugins)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need the feature flag for this.

// maybe there's a semver package to check this

// version >= 4.0.0 && < 4.26.0
return (major === 4 && minor >= 0 && minor < 26) || (minor === 26 && patch === 0)
Copy link

Choose a reason for hiding this comment

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

I think there's a @npm/semver package used in this repo so better to use that?

// Think this semver range is correct for >= 4.0.0 but < 4.26.0
semver.satisfies('4.0.0 - 4.26.0')

Copy link
Member

Choose a reason for hiding this comment

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

Make sure you set includePrerelease to true

*
* @throws Error Throws an error if the Next runtime is >= 4.0.0 || < 4.26.0
*/
export const logOutdatedPlugins = function (logs, outdatedPlugins) {
Copy link

Choose a reason for hiding this comment

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

Name slightly misleading now that it throws too?

*
* @throws Error
*/
const throwIfOutdatedNextRuntime = function (outdatedPlugins) {
Copy link
Member

Choose a reason for hiding this comment

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

Even though this is just Next for now, it would be better if this had a generic name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I was just being explicit as I thought this was a temporary one off.

@nickytonline
Copy link
Contributor Author

Closed in favour of #4672.

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.

None yet

4 participants