-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
refactor: update future options #18011
Conversation
Run & review this pull request in StackBlitz Codeflow. |
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 like the flat list, and also the new names for the sections in the docs. I think we can try this out, it feels more flexible in a way that we can add later on other kind of changes here (that maybe aren't about a deprecation but a change in the way some API works).
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.
Yeah, I like that. I see later we could potentially have error
for some of them to throw error with stacktrace etc.
Do you think we should have an option to turn on all? Like this maybe:
future: {
'*': 'warn'
}
One thing we were discussing is that maybe we could have |
I'm fine with that too and we can iterate on it later.
I'm fine supporting that too. I intentionally didn't include supporting it here since I wonder if it's often users want this. If we add a new option to |
Description
From #16471 (comment) (removed
false
because it's confusing)This change is what I figured
future
options could be. We can discuss whether this is what we want or not here.My idea is that:
future
options contain a list of top-level properties (feature change names), and users have a choice to opt-in to that breaking change.future.<name>?: "warn"
orfuture.<name>?: true
"warn"
variant is used only when we're unsure this is a change we'll land. Users can try out by opting to"warn"
.true
variant is used when we're sure of this change, and you can opt into this breaking change now. Warnings are already emitted regardless of the option, buttrue
would make them hard errors.Reading the major changes documentation, I could probably see why it landed with
future.deprecationWarnings
in the first place, so this PR may make that page a little odd. My suggestion (if we go along with this) is that in the major changes page:"warn"
, put into the "Future" section (Rename as "Considering" to not overload the future term?)true
, put into the "Current" section