[FEATURE ember-route-serializers] Introduce serialize method to Router DSL options and deprecate Route#serialize

[trentmwillis]

Implementation of emberjs/rfcs#120.

TODO:

• Add entry to FEATURES.md
• Add entry to emberjs/website deprecation guide for ember-routing.serialize-function (and update deprecate calls here to include the url) [Add to deprecation guide website#2557]
• Review documentation, and determine update needs for guides and API docs (making an issue/PR in emberjs/guides for changes needed there, and an issue in emberjs/ember.js with a list of updates needed here when the feature is enabled). [Update documentation to include route-serializer info guides#1380, Update documentation for route-serializers #13360]

[trentmwillis]

Updated to separate the serializefunctions from the route definitions.

[mixonic]

Usually we just prefix these tests with DEPRECATED:

[mixonic]

This is a good PoC @trentmwillis! A requisite next step would be this needing a feature-flag, prefixed commit etc. See contributing guides on adding a feature and CONTRIBUTING.md.

It seems like @rwjblue would also like to see the testing coverage over both features be the same, which means either duplicating the tests or better yet coming up with a good abstraction to help test the feature. Other thoughts @rwjblue? Likely @stefanpenner and @dgeb want to review in detail.

[trentmwillis]

Thanks for reviewing @mixonic! Now that the RFC has been merged I'll work on formalizing this a bit more. As for the tests, I don't think there will be a good way to abstract out the new feature vs. the old but I'll take a look.

[rwjblue]

Ya, duplicating them is fairly common (since when enabled we will delete the duplication anyways).

[trentmwillis]

Bolstered tests, added a feature flag, and prefixed the commit. I also cleaned up some usage of serialize methods that didn't need to exist. Let me know if anything else needs to happen.

[homu]

☔ The latest upstream changes (presumably #13144) made this pull request unmergeable. Please resolve the merge conflicts.

[dgeb]

Adding the deprecation message as a second argument to expectDeprecation will ensure that we're catching the right deprecation.

[dgeb]

Nice work @trentmwillis - LGTM except that one nit 👍

[trentmwillis]

@dgeb updated with your suggestion. However, it's failing but says the actual and expected values are the same, so I don't know what to do.

[dgeb]

@trentmwillis ok thanks - will investigate.

[homu]

☔ The latest upstream changes (presumably #13285) made this pull request unmergeable. Please resolve the merge conflicts.

[rwjblue]

However, it's failing but says the actual and expected values are the same, so I don't know what to do.

The message says:

Failed assertion: Did not receive failing Ember.deprecate call matching 'Defining a serialize function on route 'posts' is deprecated. Instead, define it in the router's map as an option.', last was success with 'Defining a serialize function on route 'posts' is deprecated. Instead, define it in the router's map as an option.', expected: true, but was: false

I believe it is saying that deprecate was called with the right string, but the test argument was false instead of true.

[trentmwillis]

Ah okay, that makes sense. Was not interpreting it that way. I'll look into that sometime soon.

[trentmwillis]

@rwjblue @dgeb rebased and all green now.

[rwjblue]

@trentmwillis - Awesome! I added the following items to a checklist in the main PR description:

• Add entry to FEATURES.md
• Add entry to emberjs/website deprecation guide for ember-routing.serialize-function (and update deprecate calls here to include the url)
• Review documentation, and determine update needs for guides and API docs (making an issue/PR in emberjs/guides for changes needed there, and an issue in emberjs/ember.js with a list of updates needed here when the feature is enabled).

[trentmwillis]

Sounds good, I'll try to get to those sometime this weekend.

[rwjblue]

I believe the API docs for Route.prototype.serialize need to be updated, likely in a separate PR that we can merge when we enable the feature.

[trentmwillis]

Yeah, I was going to look at the API docs tomorrow to identify what all would need to be updated. Only things I could think of were Route#serialize and, potentially, RouterDSL, but I don't think that is actually documented anywhere.

[rwjblue]

Yeah, I suspect it is documented in Ember.Router.prototype.map or the Ember.Router docs, but I haven't looked recently. We should end up with it documented somewhere in the API docs if it isn't already.

[trentmwillis]

@rwjblue I've opened a PR for the API docs update, which I believe was the last pertinent documentation change.

[rwjblue]

Awesome work @trentmwillis, thanks for keeping on this!

[xg-wang]

@trentmwillis @rwjblue I saw #13360 was closed. Is it currently recommended to define serialize on Route class or options hash? The only documentation I can find is at Route class https://api.emberjs.com/ember/3.22/classes/Route/methods/serialize?anchor=serialize

[rwjblue]

Ya, current recommendation is for route class. Though this (somewhat fundamentally) means that lazily loaded routes cannot customize their serialization behavior. Ideally you just shouldn't need to customize...