Optional framework inference #4788

nathanhammond · 2023-05-03T12:52:01Z

This makes it possible to opt out of framework inference by specifying --framework-inference=false on the command line. This is designed to interact gracefully with --env-mode=strict as a separate-but-related knob to turn for control over environment variables.

vercel · 2023-05-03T12:52:10Z

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

Name	Status	Preview	Comments	Updated (UTC)
examples-cra-web	🔄 Building (Inspect)			May 8, 2023 2:36am
examples-vite-web	🔄 Building (Inspect)			May 8, 2023 2:36am
turbo-site	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 8, 2023 2:36am

8 Ignored Deployments

Name	Status	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	May 8, 2023 2:36am
examples-designsystem-docs	⬜️ Ignored (Inspect)	May 8, 2023 2:36am
examples-gatsby-web	⬜️ Ignored (Inspect)	May 8, 2023 2:36am
examples-kitchensink-blog	⬜️ Ignored (Inspect)	May 8, 2023 2:36am
examples-native-web	⬜️ Ignored (Inspect)	May 8, 2023 2:36am
examples-nonmonorepo	⬜️ Ignored (Inspect)	May 8, 2023 2:36am
examples-svelte-web	⬜️ Ignored (Inspect)	May 8, 2023 2:36am
examples-tailwind-web	⬜️ Ignored (Inspect)	May 8, 2023 2:36am

vercel · 2023-05-03T12:52:11Z

@nathanhammond is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

nathanhammond

Reviewer's Guide! Use ?w=1, commit-by-commit is valid.

nathanhammond · 2023-05-03T12:56:23Z

cli/internal/graph/graph.go

 			packageTask,
 			taskGraph.DownEdges(taskID),
-			logger,
+			frameworkInference,


This is a global toggle; the only way to make it not a global toggle is to put it into turbo.json and that's really a "workspace config" issue for whenever we circle back to improvements in that space.

This is explicitly intended to enable the advanced use case of:

turbo run build --env-mode=strict --framework-inference=false

nathanhammond · 2023-05-03T12:57:15Z

cli/internal/run/dry_run.go

+			if rs.Opts.runOpts.FrameworkInference {
+				taskSummary.Framework = runsummary.NoFrameworkDetected
+			} else {
+				taskSummary.Framework = runsummary.FrameworkDetectionSkipped
+			}


This clarifies the output in the --framework-inference=false use case.

nathanhammond · 2023-05-03T12:59:11Z

cli/internal/run/global_hash.go

@@ -94,6 +97,11 @@ func calculateGlobalHashFromHashableInputs(full GlobalHashableInputs) (string, e
 			return newGlobalHash(full)
 		}

+		// If you tell us not to infer framework you get the new hash.
+		if full.frameworkInference == false {
+			return newGlobalHash(full)


This is an early opt-in to the "HEAD" global hash. It should impact rounds-to-zero folks.

mehulkar

Wanted a conversation on adding the skipped label into real run summaries before ✅ , but code looks good to me other than that.

mehulkar · 2023-05-03T16:29:33Z

cli/internal/runsummary/format_json.go

+	GlobalHashSummary  *GlobalHashSummary `json:"globalCacheInputs"`
+	Packages           []string           `json:"-"`
+	EnvMode            util.EnvMode       `json:"envMode"`
+	FrameworkInference bool               `json:"frameworkInference"`


Starting to think we're eventually just going to want something like:

Config runOpts `json:"config"`

Yeah, and I've got another one coming in dotEnv. I'm not excited about making a backward-incompatible change to the object shape right now, though. That should come with a minor at least.

It could be backwards compatible by duplicating whatever else you're thinking of in two places. There are other things that fall into that boat. That said, I'm not even sure we want all of runOpts, or in the form they exist today. So this is not a change we need in this PR right now

mehulkar · 2023-05-03T16:34:09Z

cli/internal/runsummary/run_summary.go

+	GlobalHashSummary  *GlobalHashSummary `json:"globalCacheInputs"`
+	Packages           []string           `json:"packages"`
+	EnvMode            util.EnvMode       `json:"envMode"`
+	FrameworkInference bool               `json:"frameworkInference"`


Is there any chance we'll add more inference? If so, might makes sense to nest this as inference.framework?

After how this went I'm expecting it to be extremely limited, and likely opted-into using improved extends instead.

(We're already discussing switching the default on this flag in the next major, and then I'm going to propose removing the flag + feature in the major after that. But better extends is a prerequisite for that.)

cli/internal/util/run_opts.go

docs/pages/repo/docs/reference/command-line-reference.mdx

mehulkar · 2023-05-03T16:38:45Z

turborepo-tests/integration/tests/_fixtures/framework_inference/.gitignore

Kinda wish we could use one of the examples for this test instead of a pseudo next-app fixture.

Examples aren't intended to be static for testing certain code paths, though. They exist to solve user-facing problems and are expected to change independently from the tests. I see this more as a unit test written in a way that works for both Go and Rust.

We can and should do more "e2e" type testing against the examples, but nowhere else in integration do we have anything reach out to ../examples.

mehulkar · 2023-05-03T16:53:35Z

cli/internal/run/dry_run.go

+			if rs.Opts.runOpts.FrameworkInference {
+				taskSummary.Framework = runsummary.NoFrameworkDetected
+			} else {
+				taskSummary.Framework = runsummary.FrameworkDetectionSkipped


The only reason MissingFrameworkLabel is only in dry run, is because it was already there and I didn't want to put it in Real Run summaries. I can't remember why I made that decision though. Maybe I thought that framework: "" was just as clear as a placeholder, so I didn't want a placeholder in a new place?

For skipped inference, maybe we do want it to say "skipped" even in real run summaries, to differentiate from framework: ""?

Or you could make the case that since we have the frameworkInference: false at the top level of the summary, you could... uhhh... infer the meaning of framework: "" in each task?

I guess I don't have a strong enough opinion, because while leaving this comment, I'm leaning towards adding the Missing and skipped labels into real run summaries, whereas I explicitly decided not to add it before. What are your thoughts?

There are two ways to derive this information:

.tasks[].framework

.frameworkInference

And the consequences of the configuration show up in run summary as:

.environmentVariables.inferred

We don't need it; it's derivable, and having an arbitrary string slug as part of an API feels bad. I added this to the PR primarily because I didn't want people getting NoFrameworkDetected when it was actually skipped.

The other legit option is to remove it from dry run, but that's an API change and should be held to at least 1.10.

(In other words: rather than adding it to run summary, I'd pitch removing it from dry run.)

In a post-Go world we could do Option<Framework> which would also easily serialize to null or slug, though since both "" and null are falsey it's not a big difference. (though it may communicate more-clearly at a glance)

You can omitempty to differentiate too.

I'm ok with the direction of removing from dry run, and in that case it's fine to only add this differentiation there.

I want the field to always be present though! lots of things blow up on missing keys.

nathanhammond requested a review from mehulkar May 3, 2023 12:52

nathanhammond requested a review from a team as a code owner May 3, 2023 12:52

nathanhammond requested a review from arlyon May 3, 2023 12:52

turbo-orchestrator bot added area: docs Improvements or additions to documentation team: turborepo labels May 3, 2023

nathanhammond commented May 3, 2023

View reviewed changes

nathanhammond force-pushed the optional-framework-inference branch 2 times, most recently from e5dede5 to 51e0c8c Compare May 3, 2023 13:19

nathanhammond mentioned this pull request May 3, 2023

No env var inference in strict mode. #4527

Closed

vercel bot deployed to Preview – turbo-site May 3, 2023 13:21 View deployment

nathanhammond force-pushed the optional-framework-inference branch from 51e0c8c to cc8a33b Compare May 3, 2023 13:42

vercel bot deployed to Preview – turbo-site May 3, 2023 13:43 View deployment

mehulkar reviewed May 3, 2023

View reviewed changes

nathanhammond requested a review from mehulkar May 4, 2023 05:29

vercel bot deployed to Preview – turbo-site May 4, 2023 05:30 View deployment

nathanhammond force-pushed the optional-framework-inference branch from c5d4c61 to 4b5fa93 Compare May 4, 2023 05:34

vercel bot deployed to Preview – turbo-site May 4, 2023 05:37 View deployment

nathanhammond force-pushed the optional-framework-inference branch from 4b5fa93 to f8fe13e Compare May 4, 2023 05:59

vercel bot deployed to Preview – turbo-site May 4, 2023 06:00 View deployment

mehulkar approved these changes May 4, 2023

View reviewed changes

mehulkar mentioned this pull request May 4, 2023

Stabilize --env-mode #4739

Merged

nathanhammond added the pr: automerge Kodiak will merge these automatically after checks pass label May 5, 2023

nathanhammond force-pushed the optional-framework-inference branch from f8fe13e to 4b40259 Compare May 5, 2023 05:17

vercel bot deployed to Preview – turbo-site May 5, 2023 05:19 View deployment

nathanhammond added 3 commits May 8, 2023 10:30

Optional framework inference

ba00215

Add docs.

c8729e8

Add integration test for framework inference opt out.

27ecf8a

nathanhammond force-pushed the optional-framework-inference branch from 4b40259 to 27ecf8a Compare May 8, 2023 02:30

vercel bot deployed to Preview – turbo-site May 8, 2023 02:36 View deployment

kodiakhq bot merged commit e942032 into vercel:main May 8, 2023

nathanhammond deleted the optional-framework-inference branch May 8, 2023 17:30

renovate bot mentioned this pull request May 12, 2023

chore(deps): update all non-major dependencies zenlinkpro/zenlink-interface#89

Closed

1 task

renovate bot mentioned this pull request Jul 3, 2023

chore(Renovate): ⬆️ Update project monorepo tools Adchitects/configs#60

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optional framework inference #4788

Optional framework inference #4788

nathanhammond commented May 3, 2023

vercel bot commented May 3, 2023 •

edited

Loading

vercel bot commented May 3, 2023

nathanhammond left a comment

nathanhammond May 3, 2023

nathanhammond May 3, 2023 •

edited

Loading

nathanhammond May 3, 2023

mehulkar left a comment

mehulkar May 3, 2023

nathanhammond May 4, 2023 •

edited

Loading

mehulkar May 4, 2023 •

edited

Loading

mehulkar May 3, 2023

nathanhammond May 4, 2023

nathanhammond May 5, 2023

mehulkar May 3, 2023

nathanhammond May 4, 2023

mehulkar May 3, 2023

nathanhammond May 4, 2023

nathanhammond May 4, 2023

nathanhammond May 4, 2023

mehulkar May 4, 2023

nathanhammond May 5, 2023

Optional framework inference #4788

Optional framework inference #4788

Conversation

nathanhammond commented May 3, 2023

vercel bot commented May 3, 2023 • edited Loading

vercel bot commented May 3, 2023

nathanhammond left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathanhammond May 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehulkar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nathanhammond May 4, 2023 • edited Loading

Choose a reason for hiding this comment

mehulkar May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented May 3, 2023 •

edited

Loading

nathanhammond May 3, 2023 •

edited

Loading

nathanhammond May 4, 2023 •

edited

Loading

mehulkar May 4, 2023 •

edited

Loading