Optional framework inference

[nathanhammond]

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]

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	Preview	Comments	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]

@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]

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]

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

[nathanhammond]

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]

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

	Config runOpts               `json:"config"`

[nathanhammond]

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.

[mehulkar]

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]

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

[nathanhammond]

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

[nathanhammond]

(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.)

[mehulkar]

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

[nathanhammond]

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]

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?

[nathanhammond]

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.

[nathanhammond]

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

[nathanhammond]

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)

[mehulkar]

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.

[nathanhammond]

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