-
Notifications
You must be signed in to change notification settings - Fork 5.6k
feat: Codegen feature flags #28920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Codegen feature flags #28920
Conversation
"sloppy-imports", | ||
"lockfile-v5", | ||
]) | ||
.chain(["fmt-component", "fmt-sql", "npm-lazy-caching", "npm-patch"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These few were never defined as unstable flags (ie. only available as options to unstable
key in deno.jsonc
file). We should decide if we want to make them flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will address it in a follow up PR
runtime/flags/flags.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should decide how we're gonna handle them once a feature is promoted to stable - are the flags getting:
a) hidden and have no effect
b) hidden and printing warning
c) removed and crashing previous invocations
d) once promoted an opposite (--no-<name>
) flag is added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer c because its easiest
@@ -6124,6 +6044,7 @@ fn unstable_args_parse( | |||
flags.unstable_config.legacy_flag_enabled = true; | |||
} | |||
|
|||
// TODO(bartlomieju): this should be factored out since these are configured via UNSTABLE_GRANULAR_FLAGS | |||
flags.unstable_config.bare_node_builtins = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like UnstableConfig
should be codegened as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to do it in a follow up PR
The JSON schema for feature flags ( deno/cli/schemas/config-file.v1.json Lines 594 to 615 in dbb5373
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This commit adds "deno_features" crate that contains definitions of all unstable features in Deno.
Based on these definitions, both Rust and JS code is generated ensuring that the two are always
in sync.
In addition some of flag handling was rewritten to use the generated definitions, instead
of hand rolling these flag definitions.