Skip to content

Commit

Permalink
[CLEANUP] Finish cleaning up Location interface
Browse files Browse the repository at this point in the history
- Remove `detect` and `implementation` from interface and its
  implementations.
- Just use normal JS property setters instead of `set`.
- Improve type safety in several spots.
  • Loading branch information
chriskrycho committed Mar 21, 2023
1 parent 87a2d97 commit 5bf26aa
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 39 deletions.
5 changes: 2 additions & 3 deletions packages/@ember/routing/hash-location.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import EmberObject, { set } from '@ember/object';
import EmberObject from '@ember/object';
import { bind } from '@ember/runloop';
import type { Location as EmberLocation, UpdateCallback } from '@ember/routing/location';
import { getHash } from './lib/location-utils';
Expand Down Expand Up @@ -36,14 +36,13 @@ import { getHash } from './lib/location-utils';
@protected
*/
export default class HashLocation extends EmberObject implements EmberLocation {
implementation = 'hash';
_hashchangeHandler?: EventListener;

private _location?: Location;
declare location: Location;

init(): void {
set(this, 'location', this._location || window.location);
this.location = this._location ?? window.location;
this._hashchangeHandler = undefined;
}

Expand Down
23 changes: 15 additions & 8 deletions packages/@ember/routing/history-location.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import EmberObject, { set } from '@ember/object';
import EmberObject from '@ember/object';
import { assert } from '@ember/debug';
import type { Location as EmberLocation, UpdateCallback } from '@ember/routing/location';
import { getHash } from './lib/location-utils';

Expand Down Expand Up @@ -58,10 +59,11 @@ function _uuid() {
@protected
*/
export default class HistoryLocation extends EmberObject implements EmberLocation {
// SAFETY: both of these properties initialized via `init`.
declare location: Location;
declare baseURL: string;

history?: any;
history?: Window['history'];

implementation = 'history';
_previousURL?: string;
Expand All @@ -75,6 +77,7 @@ export default class HistoryLocation extends EmberObject implements EmberLocatio
@private
*/
rootURL = '/';

/**
@private
Expand All @@ -95,8 +98,8 @@ export default class HistoryLocation extends EmberObject implements EmberLocatio
baseURL = base.getAttribute('href') ?? '';
}

set(this, 'baseURL', baseURL);
set(this, 'location', this.location || window.location);
this.baseURL = baseURL;
this.location = this.location ?? window.location;

this._popstateHandler = undefined;
}
Expand All @@ -108,8 +111,8 @@ export default class HistoryLocation extends EmberObject implements EmberLocatio
@method initState
*/
initState(): void {
let history = this.history || window.history;
set(this, 'history', history);
let history = this.history ?? window.history;
this.history = history;

let { state } = history;
let path = this.formatURL(this.getURL());
Expand Down Expand Up @@ -157,6 +160,7 @@ export default class HistoryLocation extends EmberObject implements EmberLocatio
@param path {String}
*/
setURL(path: string): void {
assert('HistoryLocation.history is unexpectedly missing', this.history);
let { state } = this.history;
path = this.formatURL(path);

Expand All @@ -174,6 +178,7 @@ export default class HistoryLocation extends EmberObject implements EmberLocatio
@param path {String}
*/
replaceURL(path: string): void {
assert('HistoryLocation.history is unexpectedly missing', this.history);
let { state } = this.history;
path = this.formatURL(path);

Expand All @@ -192,7 +197,8 @@ export default class HistoryLocation extends EmberObject implements EmberLocatio
pushState(path: string): void {
let state = { path, uuid: _uuid() };

this.history.pushState(state, null, path);
assert('HistoryLocation.history is unexpectedly missing', this.history);
this.history.pushState(state, '', path);

// used for webkit workaround
this._previousURL = this.getURL();
Expand All @@ -208,7 +214,8 @@ export default class HistoryLocation extends EmberObject implements EmberLocatio
replaceState(path: string): void {
let state = { path, uuid: _uuid() };

this.history.replaceState(state, null, path);
assert('HistoryLocation.history is unexpectedly missing', this.history);
this.history.replaceState(state, '', path);

// used for webkit workaround
this._previousURL = this.getURL();
Expand Down
20 changes: 0 additions & 20 deletions packages/@ember/routing/location.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,11 @@
Each location implementation must provide the following methods:
* `implementation`: returns the string name used to reference the implementation.
* `getURL`: returns the current URL.
* `setURL(path)`: sets the current URL.
* `replaceURL(path)`: replace the current URL (optional).
* `onUpdateURL(callback)`: triggers the callback when the URL changes.
* `formatURL(url)`: formats `url` to be placed into `href` attribute.
* `detect()` (optional): instructs the location to do any feature detection
necessary. If the location needs to redirect to a different URL, it
can cancel routing by setting the `cancelRouterSetup` property on itself
to `false`.
Calling `setURL` or `replaceURL` will not trigger onUpdateURL callbacks.
Expand Down Expand Up @@ -65,15 +60,6 @@
@public
*/
export interface Location {
/**
* Returns the string name used to reference the implementation.
*
* @property implementation
* @type String
* @public
*/
implementation: keyof Registry & string;

/**
* If the location needs to redirect to a different URL, it can cancel routing
* by setting the `cancelRouterSetup` property on itself to false.
Expand Down Expand Up @@ -128,12 +114,6 @@ export interface Location {
* @param {String} url the url to format
*/
formatURL(url: string): string;
/**
* Instructs the location to do any feature detection necessary. If the
* location needs to redirect to a different URL, it can cancel routing by
* setting the cancelRouterSetup property on itself to false.
*/
detect?(): void;

initState?(): void;
destroy(): void;
Expand Down
15 changes: 8 additions & 7 deletions packages/@ember/routing/none-location.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import EmberObject, { set } from '@ember/object';
import EmberObject from '@ember/object';
import { assert } from '@ember/debug';
import type { Location as EmberLocation, UpdateCallback } from '@ember/routing/location';

Expand All @@ -21,8 +21,7 @@ import type { Location as EmberLocation, UpdateCallback } from '@ember/routing/l
@protected
*/
export default class NoneLocation extends EmberObject implements EmberLocation {
declare updateCallback: UpdateCallback;
implementation = 'none';
updateCallback?: UpdateCallback;

// Set in reopen so it can be overwritten with extend
declare path: string;
Expand Down Expand Up @@ -77,7 +76,7 @@ export default class NoneLocation extends EmberObject implements EmberLocation {
@param path {String}
*/
setURL(path: string): void {
set(this, 'path', path);
this.path = path;
}

/**
Expand All @@ -101,8 +100,10 @@ export default class NoneLocation extends EmberObject implements EmberLocation {
@param url {String}
*/
handleURL(url: string): void {
set(this, 'path', url);
this.updateCallback(url);
this.path = url;
if (this.updateCallback) {
this.updateCallback(url);
}
}

/**
Expand All @@ -114,7 +115,7 @@ export default class NoneLocation extends EmberObject implements EmberLocation {
@private
@method formatURL
@param url {String}
@param {String} url
@return {String} url
*/
formatURL(url: string): string {
Expand Down
1 change: 0 additions & 1 deletion tests/docs/expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ module.exports = {
'helperContainer',
'htmlSafe',
'if',
'implementation',
'in-element',
'includes',
'incrementProperty',
Expand Down

0 comments on commit 5bf26aa

Please sign in to comment.