feat: add rome_flags crate for feature flags

[yassere]

Summary

This is a simple implementation of feature flags based on the discussion in #2471.

This PR:

• Adds a rome_flags crate with a declare_feature_flags macro
• Adds an --unstable arg to the CLI that enables all feature flags
• Adds an unstable setting for VS Code and LSP that enables all feature flags

This PR no longer contains a --features option that allows toggling feature flags individually, but it’s still useful for the feature flags themselves to have unique names. It helps associate gated code paths with the features that added them so that they're easier to clean up when features are stabilized. It also allows for in-code toggling during development.

In the future, we can consider a way to allow individual feature toggling, ideally with file-based config support.

Declaring feature flags

declare_feature_flags!(
    /// A feature that is not finished yet
    unfinished_feature,
    /// A new unstable approach to parsing
    modified_parsing
)

Checking feature flags

if rome_flags::unstable().unfinished_feature() {
    // Do something
}

Examples

Edit: The examples below worked when this PR was a draft that contained some example changes to object and assignment expression formatting. That dummy code was removed in order to ready this PR for review.

With a file foo.js containing:

foo = {test: 42}
bar = {a: "this is a very long string with many words", b: "this is another very long string to cause this object to wrap"}

Running rome format foo.js --write results in:

foo = { test: 42 };
bar =
	{
		a: "this is a very long string with many words",
		b: "this is another very long string to cause this object to wrap",
	};

Formatting with --features new_linebreaking results in:

foo = { test: 42 };
bar = {
	a: "this is a very long string with many words",
	b: "this is another very long string to cause this object to wrap",
};

Formatting with --features new_spacing results in:

foo = {test: 42};
bar =
	{
		a: "this is a very long string with many words",
		b: "this is another very long string to cause this object to wrap",
	};

Formatting with --experimental or --features new_spacing,new_linebreaking both result in:

foo = {test: 42};
bar = {
	a: "this is a very long string with many words",
	b: "this is another very long string to cause this object to wrap",
};

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Pages

Latest commit:	ddaad88
Status:	✅  Deploy successful!
Preview URL:	https://7d2584ac.tools-8rn.pages.dev

View logs

[github-actions]

Parser conformance results on ubuntu-latest

js/262

Test result	main count	This PR count	Difference
Total	45878	45878	0
Passed	44938	44938	0
Failed	940	940	0
Panics	0	0	0
Coverage	97.95%	97.95%	0.00%

jsx/babel

Test result	main count	This PR count	Difference
Total	39	39	0
Passed	36	36	0
Failed	3	3	0
Panics	0	0	0
Coverage	92.31%	92.31%	0.00%

ts/babel

Test result	main count	This PR count	Difference
Total	588	588	0
Passed	519	519	0
Failed	69	69	0
Panics	0	0	0
Coverage	88.27%	88.27%	0.00%

ts/microsoft

Test result	main count	This PR count	Difference
Total	16257	16257	0
Passed	12391	12391	0
Failed	3866	3866	0
Panics	0	0	0
Coverage	76.22%	76.22%	0.00%

[github-actions]

Playground for commit 1e1dc61

[yassere]

This approach might not play nicely with the idea of a long-running Rome server because the feature flags can only be set once.

[leops]

The first idea that comes to mind to allow the set of enabled features to be modified at runtime would be to have the features declared as a bitflags! and stored in a AtomicU*

[ematipico]

My two cents, I like the bitflags approach. We use it in some other places of the code base, so it's a familiar pattern we already use.

[yassere]

That’s a good approach for allowing the global state to be modified at runtime, but there’s still a question of whether or not global state is going to be compatible with our future architecture. If we have a long-running server that can be queried simultaneously by multiple CLI instances, then we don’t want each CLI invocation to be able to change the server's feature flags.

@leops Do you think it would be fine to stick with global state for now as a minimal solution, or will that cause us problems later if we need to move away from it? Also, if we switch to bitflags, is there a good way to avoid having to manually include the bit values (1 << x) for each flag?

[leops]

Sorry it took me some time to figure it out, but yes there is a way to let the compiler automatically generate a value for each flag by using the discriminant of an intermediate enum:

macro_rules! declare_feature_flags {
    ( $( $(#[doc = $doc:tt])* $feature:ident ),* )=> {
        #[allow(non_camel_case_types)]
        enum FlagValues {
            $( $feature, )*
        }
        
        bitflags! {
            #[derive(Default)]
            /// State of all feature flags
            pub struct FeatureFlags: u32 {
                $( const $feature = 1 << FlagValues::$feature as u32; )*
            }
        }
    };
}

Overall I think the way we handle global state is something we'll revisit when we do move to having a persistent daemon, so for the time being I'm not too worried about storing these in a global state (and maybe in the meantime the Rust language will have gained some interesting features to help with that)

[yassere]

Thanks! For now, I'll merge this PR as-is so that I can unblock Ema's unstable formatting work. And then I'll follow up later with a PR to change the FeatureFlags implementation.

[ematipico]

Thank you Yasser! I'd say we can polish up this PR and merge it, IMHO. It summarizes the discussion we had, and we can expand it later if we need.

[ematipico]

As discussed in the discussion, maybe we can keep this commented out for now. In the future people will be able to opt-in, but that would require more documentation from our end, which is not a priority now.

[yassere]

Sure. We can decide later how we want to be able to toggle features.

[ematipico]

My two cents, I like the bitflags approach. We use it in some other places of the code base, so it's a familiar pattern we already use.

[yassere]

This PR currently uses --experimental as the CLI flag that enables all features, but that falls on the wrong side of the "unfinished" vs "experimental" distinction that Micha made here and would make it harder to clarify that later.

I don't know that --unfinished is great as a CLI flag though. Any other suggestions?
--unstable? --prerelease? --wip?

[ematipico]

My initial proposal was for the use of the word unstable, with the thought that eventually it will be stable.

Experimental gives me vibes of something that might be dropped because "the experiment failed", which is not what we want.

[yassere]

Made some changes:

• Removed the --features cli arg but left in the FromStr impl for FeatureFlags for now.
• Renamed --experimental to --unstable
• Added unstable setting for VS Code and LSP.
• Flags are now checked using a method instead of field access to facilitate changing the flag implementation later
• Renamed rome_features::flags() -> rome_flags::unstable(). Later, we can possibly add rome_flags::experimental() to differentiate between unstable and experimental features.

Checking for a feature flag now looks like this:

if rome_flags::unstable().unfinished_feature() {
    // Do something
}