Add ability to define a globalEnv key in turbo.json #1950

mehulkar · 2022-09-13T21:57:44Z

This simplifies the globalDependencies key, so it can only
be responsible for file declarations and not need a microsyntax for env
vars ($ prefixes).

Review Notes

This PR is stacked on top of Add ability to declare a env key in each pipeline task #1970, so review that first!
Thanks to @gsoltis for walking me through the Golang for this, and thanks to @adrianbw for donut pairing!

vercel · 2022-09-13T21:57:49Z

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

Name	Status	Preview	Updated
turbo-site	✅ Ready (Inspect)	Visit Preview	Sep 15, 2022 at 7:45AM (UTC)

mehulkar · 2022-09-13T22:16:04Z

cli/internal/run/global_hash.go

-	// Calculate the global hash
-	globalDeps := make(util.Set)
-
+func calculateGlobalHash(rootpath turbopath.AbsolutePath, rootPackageJSON *fs.PackageJSON, pipeline fs.Pipeline, envVarDependencies []string, globalFileDependencies []string, packageManager *packagemanager.PackageManager, logger hclog.Logger, env []string) (string, error) {


I don't love that a new argument was added, would it be a good/bad idea to pass in the full turboJSON representation or does idiomatic Go prefer more arguments over unnecessary data passing?

I love that there is a new argument here, it's super clear that it's one of the inputs to the function. 🚀

cli/internal/fs/turbo_json.go

mehulkar · 2022-09-14T21:26:54Z

cli/internal/fs/turbo_json_test.go

+
+	// check global env vars also
+	assert.EqualValues(t, sortedArray([]string{"FOO", "BAR", "BAZ"}), sortedArray(turboJSON.EnvVarDependencies))
+	assert.EqualValues(t, sortedArray([]string{"somefile.txt"}), sortedArray(turboJSON.GlobalFileDependencies))


There are probably more test cases I could add here (similar to above), but it would require more testdata/ folders since there is only one globalEnv/globalDeps key. If there are additional values I can add without doing that, let me know, but will defer creating more testdata folders

gsoltis · 2022-09-14T22:21:44Z

cli/internal/fs/turbo_json.go

 	err = jsonc.Unmarshal(data, &turboJSON)
+


Suggested change

cli/internal/fs/turbo_json.go

nathanhammond · 2022-09-15T05:05:57Z

cli/internal/fs/turbo_json.go

+	envVarDependencies := make(util.Set)
+	globalFileDependencies := make(util.Set)
+
+	for _, value := range raw.GlobalEnv {
+		if strings.HasPrefix(value, envPipelineDelimiter) {
+			return fmt.Errorf("You specified \"%s\" in the \"env\" key. This is treated as a literal, you may want to remove the \"%s\"", value, envPipelineDelimiter)
+		}
+
+		envVarDependencies.Add(value)
+	}
+
+	for _, value := range raw.GlobalDependencies {
+		if strings.HasPrefix(value, envPipelineDelimiter) {
+			envVarDependencies.Add(strings.TrimPrefix(value, envPipelineDelimiter))
+		} else {
+			globalFileDependencies.Add(value)
+		}
+	}


All of this could be a shared utility function across both places. (optional)

will defer this to some future time!

nathanhammond · 2022-09-15T05:08:08Z

cli/internal/run/global_hash.go

-	// Calculate the global hash
-	globalDeps := make(util.Set)
-
+func calculateGlobalHash(rootpath turbopath.AbsolutePath, rootPackageJSON *fs.PackageJSON, pipeline fs.Pipeline, envVarDependencies []string, globalFileDependencies []string, packageManager *packagemanager.PackageManager, logger hclog.Logger, env []string) (string, error) {


I love that there is a new argument here, it's super clear that it's one of the inputs to the function. 🚀

cli/internal/run/global_hash.go

cli/internal/fs/turbo_json.go

This simplifies the globalDependencies key, so it can only be responsible for file declarations and not need a microsyntax for env vars ($ prefixes).

Docs for `env` key updates - [x] #1970 - [x] #1950 - [ ] codemod - [ ] lint rule This PR will include: - guide and reference docs updates - blog post This PR will be ready for review after all the code pieces are merged. Co-authored-by: Thomas Knickman <2933988+tknickman@users.noreply.github.com>

mehulkar changed the base branch from main to mehulkar/turbo-225-use-new-env-key-in-turbojson-to-track September 13, 2022 21:57

vercel bot deployed to Preview September 13, 2022 21:58 View deployment

mehulkar force-pushed the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch from 47c06e9 to de94c7f Compare September 13, 2022 22:08

mehulkar changed the title ~~Add ability to declare a env key in each pipeline task~~ Add ability to define a globalEnv key in turbo.json Sep 13, 2022

mehulkar mentioned this pull request Sep 13, 2022

Update docs on declaring env key dependencies #1936

Merged

4 tasks

vercel bot deployed to Preview September 13, 2022 22:09 View deployment

mehulkar marked this pull request as ready for review September 13, 2022 22:13

mehulkar requested a review from a team as a code owner September 13, 2022 22:13

mehulkar requested review from jaredpalmer, gaspar09 and gsoltis and removed request for a team September 13, 2022 22:13

mehulkar commented Sep 13, 2022

View reviewed changes

mehulkar force-pushed the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch from de94c7f to 51f65f3 Compare September 13, 2022 23:06

vercel bot deployed to Preview September 13, 2022 23:07 View deployment

mehulkar mentioned this pull request Sep 14, 2022

chore(core): deprecation messages for legacy env keys #1959

Merged

mehulkar commented Sep 14, 2022

View reviewed changes

cli/internal/fs/turbo_json.go Outdated Show resolved Hide resolved

mehulkar commented Sep 14, 2022

View reviewed changes

cli/internal/fs/turbo_json.go Outdated Show resolved Hide resolved

mehulkar commented Sep 14, 2022

View reviewed changes

gsoltis added the pr: fixship A PR which is approved with notes and does not require re-review. label Sep 14, 2022

gsoltis approved these changes Sep 14, 2022

View reviewed changes

mehulkar force-pushed the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch from 51f65f3 to 4c2d596 Compare September 15, 2022 04:48

vercel bot deployed to Preview September 15, 2022 04:49 View deployment

nathanhammond approved these changes Sep 15, 2022

View reviewed changes

mehulkar force-pushed the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch from 4c2d596 to ae55621 Compare September 15, 2022 06:07

vercel bot deployed to Preview September 15, 2022 06:08 View deployment

mehulkar force-pushed the mehulkar/turbo-225-use-new-env-key-in-turbojson-to-track branch from 1edc7dd to db4736f Compare September 15, 2022 06:16

mehulkar force-pushed the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch from ae55621 to 09fa5e6 Compare September 15, 2022 06:28

vercel bot deployed to Preview September 15, 2022 06:29 View deployment

mehulkar force-pushed the mehulkar/turbo-225-use-new-env-key-in-turbojson-to-track branch 2 times, most recently from f821505 to 4c334d0 Compare September 15, 2022 07:25

mehulkar force-pushed the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch from 09fa5e6 to f63e3d8 Compare September 15, 2022 07:32

vercel bot deployed to Preview September 15, 2022 07:32 View deployment

Base automatically changed from mehulkar/turbo-225-use-new-env-key-in-turbojson-to-track to main September 15, 2022 07:44

Add ability to define a globalEnv key in turbo.json

3889133

This simplifies the globalDependencies key, so it can only be responsible for file declarations and not need a microsyntax for env vars ($ prefixes).

mehulkar force-pushed the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch from f63e3d8 to 3889133 Compare September 15, 2022 07:44

vercel bot deployed to Preview September 15, 2022 07:45 View deployment

mehulkar merged commit 7ccd926 into main Sep 15, 2022

mehulkar deleted the mehulkar/turbo-227-implement-globalenv-key-instead-of-foo branch September 15, 2022 07:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add ability to define a globalEnv key in turbo.json #1950

Add ability to define a globalEnv key in turbo.json #1950

Uh oh!

mehulkar commented Sep 13, 2022 •

edited

Loading

Uh oh!

vercel bot commented Sep 13, 2022 •

edited

Loading

Uh oh!

mehulkar Sep 13, 2022

Uh oh!

nathanhammond Sep 15, 2022

Uh oh!

Uh oh!

Uh oh!

mehulkar Sep 14, 2022

Uh oh!

gsoltis Sep 14, 2022

Uh oh!

Uh oh!

Uh oh!

nathanhammond Sep 15, 2022 •

edited

Loading

Uh oh!

mehulkar Sep 15, 2022

Uh oh!

nathanhammond Sep 15, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add ability to define a globalEnv key in turbo.json #1950

Add ability to define a globalEnv key in turbo.json #1950

Uh oh!

Conversation

mehulkar commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Notes

Uh oh!

vercel bot commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mehulkar Sep 13, 2022

Choose a reason for hiding this comment

Uh oh!

nathanhammond Sep 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mehulkar Sep 14, 2022

Choose a reason for hiding this comment

Uh oh!

gsoltis Sep 14, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nathanhammond Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mehulkar Sep 15, 2022

Choose a reason for hiding this comment

Uh oh!

nathanhammond Sep 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mehulkar commented Sep 13, 2022 •

edited

Loading

vercel bot commented Sep 13, 2022 •

edited

Loading

nathanhammond Sep 15, 2022 •

edited

Loading