-
-
Notifications
You must be signed in to change notification settings - Fork 406
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
Advance RFC #0774 "Deprecate Implicit Record Loading in Routes"
to Stage Recommended
#970
Conversation
This should probably advance since it's a deprecation and there's no further work to do here. |
Is this something we should be considering? After discussing we were unconvinced that it'd be worth the effort of creating a codemod for this. Thoughts? |
I don't feel like we need codemods for this, but I'm also not opposed. |
Has this optional feature been implemented? I don't see one in |
This is not the kind of change we use optional features for. We have the deprecation workflow for this purpose. The RFC should probably be edited to reflect this instead. |
Not sure if I agree. Without an optional feature you must implement a |
The RFC text calls for an optional feature for deliberate reasons. Without the optional feature, users who are not relying on the deprecated behavior have no way of removing the warning without writing temporary code that wouldn't actually be needed in the upcoming ember major version. There are legitimate use cases that are safe on the next Ember major that cause deprecations warnings. The optional feature lets you skip to the new implementation without waiting for the major, avoiding the need to introduce temporary code just to convince Ember your code is safe. So I agree with @jelhan's catch and this isn't ready to move to recommended with the missing optional feature. |
Status update here: we still need someone to implement the optional feature that opts people into the new behavior (of never automatically doing implicit record loading). |
Here's a draft PR for the optional-features repo: emberjs/ember-optional-features#334 Was working on this with @kategengler . She will have a PR for the ember.js repo. |
Adding `no-implicit-route-model` for emberjs/rfcs#970
Regarding the name of the new optional feature. I think |
We thought about this when we added it -- felt it was easier to have the "on" value as a true ... so to have the existing behavior the flag is off with |
Thoughts on @mansona 's comment ember-cli/ember-cli#10440 (comment) ? |
Adding `no-implicit-route-model` for emberjs/rfcs#970
Just in case anyone is following along here and didn't see my other comment here is what I'm proposing:
Does anyone object to introducing this optional feature with the new state? i.e. add
to the optional-features.json |
I agree with this; I think we should update the RFC to say the optional feature should be immediately enabled in the blueprint so that new apps do not get this old behavior, especially as it will be removed soon in 6.0. |
RFC Update here: #1010 |
Adding `no-implicit-route-model` for emberjs/rfcs#970
Advance #774 to the Recommended Stage
Summary
This pull request is advancing the RFC to the Recommended Stage.
An FCP is required before merging this PR to advance.
Recommended Stage Summary
The "Recommended" stage is the final milestone for an RFC. It provides a signal to the wider community to indicate that a feature has been put through its ecosystem paces and is ready to use.
To reach the "Recommended" stage, the following should be true:
If appropriate, the feature is integrated into the tutorial and the guides prose. API documentation is polished and updates are carried through to other areas of API docs that may not directly pertain to the feature.
If the proposal replaces an existing feature, the addon ecosystem has largely updated to work with both old and new features.
If the proposal updates or replaces an existing feature, high-quality codemods are available.
If needed, Ember debugging tools as well as popular IDE support have been updated to support the feature.
If the feature is part of a suite of features that were designed to work together for best ergonomics, the other features are also ready to be "Recommended".
Any criteria for "Recommended" for this proposal that were established in the Ready For Release stage have been met.
An FCP is required to enter this stage. Multiple RFCs may be moved as a batch into "Recommended" with the same PR.
Checklist to move to Recommended
Final Comment Period
label has been added to start the FCPCriteria for moving to Recommended (required)
A set of criteria for moving this RFC to the Recommended Stage, following release:
true
) Update optional-features.json ember-cli/ember-cli#10440