-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat(build-info): pass feature flags as json and change plugins to an object #5449
base: main
Are you sure you want to change the base?
Conversation
… object BREAKING CHANGE: Plugin build settings now use an object array, not string lists, allowing extra details like install source and if always needed at runtime.
export type BuildPlugin = { | ||
name: string | ||
/** Plugins that should be always installed */ | ||
alwaysInstall?: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a bit more context to this property? When will it be used? I don't see it being pulled in yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reference is in the PR description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we have a lot of hardcoded instances of the next runtime inside the react UI and the CLI, where we determine if it needs to be auto-installed:
https://github.com/netlify/netlify-react-ui/blob/main/apps/netlify-react-ui/src/actions/repos.ts#L219-L239
https://github.com/netlify/cli/blob/main/src/utils/init/utils.ts#L19-L43
I'm going to cut a pre-release before merging this and trying to prepare the breaking changes already upfront on the repositories.
|
BREAKING CHANGE: Plugin build settings now use an object array, not string lists, allowing extra details like install source and if always needed at runtime.
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes https://linear.app/netlify/issue/FRA-139/update-build-info-to-include-netlifynext-runtime
Currently, we have a lot of hardcoded instances of the next runtime inside the react UI and the CLI, where we determine if it needs to be auto-installed:
This makes it hard to roll out the new runtime over a central place. Therefore, I've introduced a breaking change here, making the plugins an array of objects that contains the additional information that the next runtime should always be automatically installed.
Based on the feature flag, we can now roll out the new or old runtime seamlessly with this change.
Therefore we had to change the featureflags to be a JSON of key value objects to retrieve the value of the string flag.
For us to review and ship your PR efficiently, please perform the following steps:
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.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)