feat: opt into breaking changes and pre-stable features early with turbo.json configuration
#10590
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
turbo.json configuration
turbo.json configurationturbo.json configuration
turbo.json configurationturbo.json configuration
| @@ -164,6 +164,9 @@ pub struct RawTurboJson { | |||
| #[serde(skip_serializing_if = "Option::is_none")] | |||
| pub concurrency: Option<String>, | |||
|
|
|||
| #[serde(skip_serializing_if = "Option::is_none", rename = "futureFlags")] | |||
| pub future_flags: Option<Spanned<serde_json::Value>>, | |||
There was a problem hiding this comment.
Happy to change the key name if we like something else better! Was the first thing that came to mind.
There was a problem hiding this comment.
experimental to match Next.js?
There was a problem hiding this comment.
I meant to nest keys into this futureFlags key and forgot. I intended to do:
{
"futureFlags": {
"beta: {},
"experimental": {}
}
}
What do you think of that? Notably, experimental is the name of one of our release phases, so I don't want to create confusion by ending up with some experimental.foo, while foo is actually in Beta.
There was a problem hiding this comment.
Maybe just flags then? e.g. flags.experimental.foo
There was a problem hiding this comment.
This approach would mean we would have to consciously promote a feature through its phases, though, and I don't know if we would want that. That would mean breaking changes within this flag since it would break parsing when a flag moves between phases in a user's codebase. (I am okay with this, but I could see opposition.)
There was a problem hiding this comment.
That felt too close to application feature flags (e.g. https://flags-sdk.dev/) which worries me.
There was a problem hiding this comment.
This approach would mean we would have to consciously promote a feature through its phases
Not necessarily. I'll do a quick PoC to show how we could have stable ⊆ beta ⊆ experimental "for free" so users don't need to change flag declaration as they get stabilized.
There was a problem hiding this comment.
That feels confusing as well. I might be on a version of Turborepo where Boundaries are Beta, but then we would accept the flag in the experimental key? I guess we could warn?
There was a problem hiding this comment.
I feel pretty strongly that we need to support reading beta flags from experimental path. Even with a warning it is easy enough to miss and a user not realize by upgrading turbo they unintentionally opted out of an experimental flag they had enabled because it is now in beta.
There was a problem hiding this comment.
I would say in that case we don't need to have the nested keys for phases at all. The idea there was to make things explicit so users would know what phase a feature is in, but if we accept them anywhere, then that explicitness is lost and its a layer of nesting that expresses effectively nothing.
Stick with futureFlags: {}?
There was a problem hiding this comment.
Overall looks good, only blocker is updating @turbo/types.
I did push up a commit to switch to using a FeatureFlags struct instead of a Value. This lets us rely on Biome validating the types of the flags instead of handrolling it ourselves. (Noop now since we don't have any flags in here)
|
|
||
| // `futureFlags` key is only allowed in root turbo.json | ||
| let is_workspace_config = raw_turbo.extends.is_some(); | ||
| if is_workspace_config { | ||
| if let Some(future_flags) = raw_turbo.future_flags { | ||
| let (span, text) = future_flags.span_and_text("turbo.json"); | ||
| return Err(Error::FutureFlagsInPackage { span, text }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Was trying to find how we prevent root level field usage outside of the root and suggest using that mechanism instead of this check. We don't prevent it 🙃
There was a problem hiding this comment.
I can make a utility out of this in a separate PR?
There was a problem hiding this comment.
Oh yeah, nothing to do in this PR. Just future work
### Description In #10590, I edited the JSON schema directly, which [isn't the right way](#10590 (comment)). This PR introduces a CI check that I (and others) can't make this sort of mistake in the future. ### Testing Instructions [This commit](3108ad0) shows a failure because I added a `test` key to the schema (and malformatted it in the process).
Description
We want to smooth migration paths for users when we introduce functionality with breaking changes. This can be difficult with point-in-time majors that might force a user to make many breaking changes at once.
Instead, we can introduce the concept of "future flags" to allow you to opt into breaking changes early. As a user, you get to have more control over when you adopt breaking changes. As a core team, we get more freedom to iterate, resulting in faster feedback cycles with users.
This PR only introduces the flag to the parser, not adding any flags quite yet. This key is only allowed in root
turbo.jsons.Testing Instructions
Added some tests. I also tried it out manually (and trivially). It worked how I'd hoped.