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: pass config object to devCommand step handler #6095

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Conversation

VitaliyR
Copy link
Contributor

@VitaliyR VitaliyR commented Feb 19, 2025

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes WRFL-2278

When plugins overrides the ENV via writing into args[0].netlifyConfig.build.environment, this new value is being applied to the environment.

But later on, the netlify cli, when it starts the dev server, it never passes this changed env var into it

Here is the dev Command
https://github.com/netlify/cli/blob/main/src/utils/run-build.ts#L90
startFrameworkServer passes settings, which has env but it's typically empty.

So, if I'm having a plugin which does change the env, it is never got into the user app.

To fix this, I'm just providing the arguments into the devCommand call, so then the place-of-usage (in this case - netlify-cli) can use it like that:

const devCommand = async ({ netlifyConfig }: { netlifyConfig: NetlifyConfig }) => {
  const env = {
      ...settings.env,
      ...netlifyConfig?.build.environment // now have access to this!
  };
}

Appropriate PR in CLI which supposed to use this PR - netlify/cli#7045


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)

@VitaliyR VitaliyR requested a review from a team as a code owner February 19, 2025 13:24
Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

@mrstork mrstork left a comment

Choose a reason for hiding this comment

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

Looks okay to me!

@VitaliyR VitaliyR merged commit e234d01 into main Feb 20, 2025
32 of 33 checks passed
@VitaliyR VitaliyR deleted the vitaliir/WRFL-2278 branch February 20, 2025 18:05
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.

2 participants