[CLEANUP] Remove deprecated auto-location #20406
Conversation
|
@chriskrycho If you have specific questions, or specific tasks, I can see if I may be able to tackle them, if they aren't too much work. So for example, what parts are remaining? Also, I've never written Typescript, I guess I can delete typescript 😄 but writing the code is a bit of a hurdle for me. |
|
I have, unfortunately, completely lost the thread on "what is remaining" from when I started on this many weeks ago. 😩 |
|
@sandstrom I was able to pick this back up and work on it a bit, and hit a thing which is not specified in the RFC: the exported |
chriskrycho
force-pushed
the
goodbye-autolocation
branch
from
f8649ab to
71e662a
Compare
Replace `Location` object with an interface. We no longer need the object (it does not do anything) and in fact exporting it would just troll people. However, we *do* need the interface internally, and it is useful to allow people to name the 'any Location type' type, so export that in its place. Along the way, delete a test which prevented the DI registry type from working correctly in Ember's internals, and simplify a few of the type lookups there.
chriskrycho
force-pushed
the
goodbye-autolocation
branch
from
71e662a to
72bf784
Compare
| expectTypeOf<Location['replaceURL']>().toEqualTypeOf<((url: string) => void) | undefined>(); | ||
| expectTypeOf<Location['onUpdateURL']>().toEqualTypeOf<(callback: (url: string) => void) => void>(); | ||
| expectTypeOf<Location['formatURL']>().toEqualTypeOf<(url: string) => string>(); | ||
| expectTypeOf<Location['detect']>().toEqualTypeOf<(() => void) | undefined>(); |
There was a problem hiding this comment.
I think we should remove detect too.
It was mentioned in the RFC (https://github.com/emberjs/rfcs/blob/master/text/0711-deprecate-auto-location.md#what-to-deprecate-and-later-remove) and it isn't used anywhere in the codebase, afaik.
There was a problem hiding this comment.
Ah, I missed that! Excellent.
| * location needs to redirect to a different URL, it can cancel routing by | ||
| * setting the cancelRouterSetup property on itself to false. | ||
| */ | ||
| detect?(): void; |
There was a problem hiding this comment.
detect can be removed. See the other comment.
| * @type String | ||
| * @public | ||
| */ | ||
| implementation: keyof Registry & string; |
There was a problem hiding this comment.
Don't think this is used for anything and should be removed.
The registry will be used to store different implementations, but the instances themselves doesn't need to know their implementation name. I think this is a remnant from ages ago, before ember had a registry.
There was a problem hiding this comment.
Thanks – I'll double check it!
Sounds like a good idea! I've added a few other comments above. |
Current status: in progress and needs someone else to wrap it up. This gets most of the way there, but needs the last few pieces pulled across the line. cc. @sandstrom – if you have bandwidth to drive this, please say so! I would like to have finished it, but work has been 🔥 for this whole quarter.