vercel / turborepo

Build system optimized for JavaScript and TypeScript, written in Rust
https://turbo.build/repo/docs
MIT License
26.34k stars 1.83k forks source link

Automatically depend on NEXT_PUBLIC env vars #1058

Closed jaredpalmer closed 2 years ago

jaredpalmer commented 2 years ago

Discussed in https://github.com/vercel/turborepo/discussions/981

Originally posted by **ericmatthys** March 31, 2022 Given the out of the box support with Vercel and somewhat by extension Next.js, I think it'd be worth considering assuming all `NEXT_PUBLIC` env vars affect the hash used for caching. We have quite a few and now our `turbo.json` is another place we need to track them all. Given that `NEXT_PUBLIC` env vars are used for vars that can baked into in the build, I think it'd be a safe assumption that any env var that begins with `NEXT_PUBLIC` should affect the hash. I believe this would lead to fewer issues from the Vercel community giving Turborepo a try.
MattKetmo commented 2 years ago

Hello, is this issue fixed?

I read on release 1.4 that "Turborepo will now automatically infer and include public environment variables". https://turborepo.org/blog/turbo-1-4-0#automatic-environment-variable-inclusion

However when testing looks like it still fail. Example:

web:build:
web:build: > web@1.0.0 build
web:build: > next build
web:build:
web:build: info  - Linting and checking validity of types...
web:build:
web:build: Failed to compile.
web:build:
web:build: ./src/pages/index.tsx
web:build: 4:17  Error: $NEXT_PUBLIC_MY_TOKEN is not listed as a dependency in turbo.json  turbo/no-undeclared-env-vars
web:build:
web:build: info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules
web:build: npm ERR! Lifecycle script `build` failed with error:
web:build: npm ERR! Error: command failed
web:build: npm ERR!   in workspace: web@1.0.0
web:build: npm ERR!   at location: /Users/ketmo/src/github.com/MattKetmo/snippets/apps/web
web:build: ERROR: command finished with error: command (apps/web) npm run build exited (1)

I'm not sure the issue is Turbo missing the framework detection or just the eslint rule (which then should also be fixed).

raulmabe commented 2 years ago

@MattKetmo How did you solve the issue? I'm having the same error on build on create-turbo@1.4.5

MattKetmo commented 2 years ago

for now I still set my NEXT_PUBLIC_ env vars in turbo.json

tknickman commented 2 years ago

Hey @MattKetmo / @raulmabe - yes you're right. There is a discrepancy right now between the eslint plugin and turbo.

First, you are correct, and turbo@1.4.3 or greater will automatically include any framework specific env variables so technically they do not need to be included in turbo.json. However, the eslint plugin doesn't know about this.

We did this for a few reasons:

  1. We currently don't do any version detection in the eslint plugin. So we don't know if you're running turbo 1.4.3 or greater and these can be safely excluded.
  2. There are some cases when an env var can be set after turbo has a chance to catch it (see invisible-environment-variables for more details)
  3. There is an escape hatch by:
    1. Setting the vars in turbo.json (as you're currently doing)
    2. Setting an allowList for the eslint rule (docs)

It's on our list to make this better (along with better observability into what env vars we're auto detecting / including in the hash), but we chose the "extra safe even if it's a little redundant" route for now because accidentally shipping the wrong cached build because an input was forgotten is not a position we want anyone to be in!

tknickman commented 2 years ago

All that being said, the core of this issue has been completed, so I'm going to close this one out!

jonahallibone commented 2 years ago

EDIT

I believe my issue is related to https://github.com/vercel/turborepo/issues/1964

@tknickman No matter how I add my environment variables to the turbo.json I get a lint error that they could not be found.

Error: $NEXT_PUBLIC_CONTENTFUL_SPACE_ID is not listed as a dependency in turbo.json  turbo/no-undeclared-env-vars

I've tried putting them any/everywhere I could think to

{
  "$schema": "https://turborepo.org/schema.json",
  "pipeline": {
    "build": {
      "dependsOn": ["^build"],
      "outputs": ["dist/**", ".next/**"]
    },
    "marketing#build": {
      "dependsOn": [
        "^build",
        "$GITHUB_TOKEN",
        "$SYSTEM_URL",
        "$NODE_ENV",
        "$STRIPE_API_KEY",
        "$RECAPTCHA_KEY",
        "$NEXT_MAILCHIMP_API_KEY",
        "$SENDGRID_API_KEY",
        "$CONTENTFUL_SPACE_ID",
        "$CONTENTFUL_ACCESS_TOKEN",
        "$CONTENTFUL_ENVIRONMENT",
        "$RECAPTCHA_SECRET_KEY"
      ],
      "outputs": [".next/**"]
    },
    "lint": {
      "outputs": [],
      "dependsOn": [
        "$GITHUB_TOKEN",
        "$SYSTEM_URL",
        "$NODE_ENV",
        "$STRIPE_API_KEY",
        "$RECAPTCHA_KEY",
        "$NEXT_MAILCHIMP_API_KEY",
        "$SENDGRID_API_KEY",
        "$CONTENTFUL_SPACE_ID",
        "$CONTENTFUL_ACCESS_TOKEN",
        "$CONTENTFUL_ENVIRONMENT",
        "$RECAPTCHA_SECRET_KEY"
      ]
    },
    "dev": {
      "cache": false
    }
  },
  "globalDependencies": [
    "$GITHUB_TOKEN",
    "$SYSTEM_URL",
    "$NODE_ENV",
    "$STRIPE_API_KEY",
    "$RECAPTCHA_KEY",
    "$NEXT_MAILCHIMP_API_KEY",
    "$SENDGRID_API_KEY",
    "$CONTENTFUL_SPACE_ID",
    "$CONTENTFUL_ACCESS_TOKEN",
    "$CONTENTFUL_ENVIRONMENT",
    "$RECAPTCHA_SECRET_KEY"
  ]
}

on the latest version of turbo

tknickman commented 2 years ago

@jonahallibone you will need to add the full env var to one of the dependsOn lists or globalDependencies for this to go away: So, $NEXT_PUBLIC_CONTENTFUL_SPACE_ID instead of just $CONTENTFUL_SPACE_ID

grempe commented 1 year ago

Hi @tknickman

In your comments you said:

First, you are correct, and turbo@1.4.3 or greater will automatically include any framework specific env variables so technically they do not need to be included in turbo.json. However, the eslint plugin doesn't know about this.

It's on our list to make this better (along with better observability into what env vars we're auto detecting / including in the hash)

I searched for an issue to track the improvements you mentioned to have eslint-config-turbo work properly with NEXT_PUBLIC_* environment variables, but I was unable to find one.

Can you please provide a link to the issue that tracks this, or create a new one?

Thanks