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

chore: make env mode strict by default #8182

Conversation

chris-olszewski
Copy link
Member

@chris-olszewski chris-olszewski commented May 21, 2024

Description

This change is being introduced to help users not accidentally hit cache. We've heard feedback that it's too easy to forget to add an env key, so this change will make it so your build fails when you do not have the variables needed available in the task runtime to build the package/application.

Note: This isn't foolproof, since the app/package could gracefully handle the missing variable - but it should save a lot of user pain as far as hitting unintended caches.

Commandeered from #8151

Changes default env mode to strict and remove the "infer" option

Testing Instructions

Existing test suite

(Currently some Windows integration tests are failing due to a missing env var, opening for review while I work on this)

Copy link

vercel bot commented May 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 21, 2024 11:07pm
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2024 11:07pm
rust-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2024 11:07pm
7 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview May 21, 2024 11:07pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview May 21, 2024 11:07pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview May 21, 2024 11:07pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview May 21, 2024 11:07pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview May 21, 2024 11:07pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview May 21, 2024 11:07pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview May 21, 2024 11:07pm

Copy link
Contributor

github-actions bot commented May 21, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

github-actions bot commented May 21, 2024

🟢 CI successful 🟢

Thanks

let mut pass_through_env = EnvironmentVariableMap::default();
let default_env_var_pass_through_map =
self.env_at_execution_start.from_wildcards(&[
"SHELL",
// Command Prompt casing of env variables
"APPDATA",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is necessary due to older versions of pnpm not having pnpm/npm-conf#10 which results in pnpm crashing if APPDATA isn't defined.

Technically we could have users define this instead of baking it into turbo itself, but considering the popularity of pnpm and the amount of digging it took to find that PR, it seems nice to include it.

@chris-olszewski chris-olszewski merged commit 68ab221 into turborepo_2 May 22, 2024
55 checks passed
@chris-olszewski chris-olszewski deleted the chrisolszewski/turbo-2706-make-env-mode-strict-by-default branch May 22, 2024 02:16
chris-olszewski added a commit that referenced this pull request May 22, 2024
### Description

Commandeered from #8151

Changes default env mode to strict and remove the "infer" option

### Testing Instructions

Existing test suite

(Currently some Windows integration tests are failing due to a missing
env var, opening for review while I work on this)

---------

Co-authored-by: nicholaslyang <[email protected]>
chris-olszewski added a commit that referenced this pull request May 28, 2024
### Description

Commandeered from #8151

Changes default env mode to strict and remove the "infer" option

### Testing Instructions

Existing test suite

(Currently some Windows integration tests are failing due to a missing
env var, opening for review while I work on this)

---------

Co-authored-by: nicholaslyang <[email protected]>
chris-olszewski added a commit that referenced this pull request May 29, 2024
### Description

Commandeered from #8151

Changes default env mode to strict and remove the "infer" option

### Testing Instructions

Existing test suite

(Currently some Windows integration tests are failing due to a missing
env var, opening for review while I work on this)

---------

Co-authored-by: nicholaslyang <[email protected]>
chris-olszewski added a commit that referenced this pull request May 31, 2024
### Description

Commandeered from #8151

Changes default env mode to strict and remove the "infer" option

### Testing Instructions

Existing test suite

(Currently some Windows integration tests are failing due to a missing
env var, opening for review while I work on this)

---------

Co-authored-by: nicholaslyang <[email protected]>
chris-olszewski added a commit that referenced this pull request Jun 4, 2024
### Description

Commandeered from #8151

Changes default env mode to strict and remove the "infer" option

### Testing Instructions

Existing test suite

(Currently some Windows integration tests are failing due to a missing
env var, opening for review while I work on this)

---------

Co-authored-by: nicholaslyang <[email protected]>
@andrevenancio
Copy link

What was the motivation to change how environment variables are handled? I'm failing to understand based on your changelog's on turbo 2.0 and this PR (which you link from the changelog) WHY was this necessary. Why am I allowed to build and the environment variables aren't provided by default. My app if failing on vercel.com and the environment variables are there. Why do I need to be explicit on my turbo.json file to include variable A B or C? If the env variables are defined on my project dashboard then they all should be available.

I'm running turborepo 2.0 and my build isn't working anymore. even with --env-mode=loose.

Would be great to read somewhere why this change was necessary.
Thank you

@Wizzel1
Copy link

Wizzel1 commented Jun 20, 2024

What was the motivation to change how environment variables are handled? I'm failing to understand based on your changelog's on turbo 2.0 and this PR (which you link from the changelog) WHY was this necessary. Why am I allowed to build and the environment variables aren't provided by default. My app if failing on vercel.com and the environment variables are there. Why do I need to be explicit on my turbo.json file to include variable A B or C? If the env variables are defined on my project dashboard then they all should be available.

I'm running turborepo 2.0 and my build isn't working anymore. even with --env-mode=loose.

Would be great to read somewhere why this change was necessary. Thank you

Facing the same problem. After migration my environment variables are missing. Did you find a solution?

franky47 added a commit to 47ng/nuqs that referenced this pull request Jun 26, 2024
Turbo 2 requires being explicit about the env vars consumed
in each task (not inheriting the parent tasks')
vercel/turborepo#8182
franky47 added a commit to 47ng/nuqs that referenced this pull request Jun 26, 2024
* chore: Update turborepo to v2

* chore: Fix env bursting for e2e run task

Turbo 2 requires being explicit about the env vars consumed
in each task (not inheriting the parent tasks')
vercel/turborepo#8182
@yashsway
Copy link

yashsway commented Aug 1, 2024

What was the motivation to change how environment variables are handled? I'm failing to understand based on your changelog's on turbo 2.0 and this PR (which you link from the changelog) WHY was this necessary. Why am I allowed to build and the environment variables aren't provided by default. My app if failing on vercel.com and the environment variables are there. Why do I need to be explicit on my turbo.json file to include variable A B or C? If the env variables are defined on my project dashboard then they all should be available.
I'm running turborepo 2.0 and my build isn't working anymore. even with --env-mode=loose.
Would be great to read somewhere why this change was necessary. Thank you

Facing the same problem. After migration my environment variables are missing. Did you find a solution?

Have the exact same issue as both of you @Wizzel1 @andrevenancio ! I've tried using env, globalEnv, globalPassThroughEnv, and --env-mode=loose directly and none of it is letting my CircleCI environment variables through. 😢 I know for sure that this is because of the 2.0 upgrade because it stopped working right after I upgraded Turbo to 2.0 which I've double-checked in the CircleCI workflow logs.

@chris-olszewski any idea what might be causing this?

turbo.json:

{
  "$schema": "https://turbo.build/schema.json",
  "globalDependencies": [
    "**/.env.*",
    "tsconfig.json",
    "tsconfig.base.json",
    "tsconfig.lib.json"
  ],
  "globalPassThroughEnv": ["CIRCLE_*", "VERCEL_*"],
  "tasks": {
    "build": {
      "dependsOn": ["^build"],
      "outputs": [
        "dist/**",
        "build/**",
        ".next/**",
        "!.next/cache/**",
        "lib/**"
      ]
    },
    "build:stage": {
      "dependsOn": ["^build"],
      "outputs": [
        "dist/**",
        "build/**",
        ".next/**",
        "!.next/cache/**",
        "lib/**"
      ]
    },
    "build:integration": {
      "dependsOn": ["^build"],
      "outputs": [
        "dist/**",
        "build/**",
        ".next/**",
        "!.next/cache/**",
        "lib/**"
      ]
    },
    "build:prod": {
      "dependsOn": ["^build"],
      "outputs": [
        "dist/**",
        "build/**",
        ".next/**",
        "!.next/cache/**",
        "lib/**"
      ]
    },
   // ... excluded rest for brevity
  }
}

package.json:

  "scripts": {
    "analyze": "turbo run analyze",
    "build": "turbo run build",
    "build:integration": "turbo --env-mode=loose run build:integration",
    "build:prod": "turbo --env-mode=loose run build:prod",
    "build:stage": "turbo --env-mode=loose run build:stage",
    // ... excluded rest for brevity
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants