cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(elements): simplify accessing NgElementStrategy (#36114) (#37226)

Browse Source 
Previously, we had to check whether `NgElementStrategy` had been
instantiated before accessing it. This was tedious and error prone,
since it was easy to forget to add the check in new call sites.

This commit switches to using a getter, so that the check has to be
performed in one place and is transparent to call sites (including any
future ones).

PR Close #36114

PR Close #37226
This commit is contained in:
George Kalpakas 2020-05-20 11:10:22 +03:00 committed by Kara Erickson
parent 6ac0042aef
commit 75ead94ed5
1 changed files with 23 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
packages/elements/src/create-custom-element.ts
View File

@ -136,31 +136,35 @@ export function createCustomElement<P>(
// field externs. So using quoted access to explicitly prevent renaming.
static readonly['observedAttributes'] = Object.keys(attributeToPropertyInputs);
constructor(injector?: Injector) {
super();
// Note that some polyfills (e.g. document-register-element) do not call the constructor.
// Do not assume this strategy has been created.
protected get ngElementStrategy(): NgElementStrategy {
// NOTE:
// Some polyfills (e.g. `document-register-element`) do not call the constructor, therefore
// it is not safe to set `ngElementStrategy` in the constructor and assume it will be
// available inside the methods.
//
// TODO(andrewseguin): Add e2e tests that cover cases where the constructor isn't called. For
// now this is tested using a Google internal test suite.
this.ngElementStrategy = strategyFactory.create(injector || config.injector);
if (this._ngElementStrategy === null) {
this._ngElementStrategy = strategyFactory.create(this.injector);
}
return this._ngElementStrategy;
}
private readonly injector: Injector;
private _ngElementStrategy: NgElementStrategy|null = null;
constructor(injector?: Injector) {
super();
this.injector = injector || config.injector;
}
attributeChangedCallback(
attrName: string, oldValue: string|null, newValue: string, namespace?: string): void {
if (!this.ngElementStrategy) {
this.ngElementStrategy = strategyFactory.create(config.injector);
}
const propName = attributeToPropertyInputs[attrName]!;
this.ngElementStrategy.setInputValue(propName, newValue);
}
connectedCallback(): void {
if (!this.ngElementStrategy) {
this.ngElementStrategy = strategyFactory.create(config.injector);
}
this.ngElementStrategy.connect(this);
// Listen for events from the strategy and dispatch them as custom events
@ -171,8 +175,9 @@ export function createCustomElement<P>(
}
disconnectedCallback(): void {
if (this.ngElementStrategy) {
this.ngElementStrategy.disconnect();
// Not using `this.ngElementStrategy` to avoid unnecessarily creating the `NgElementStrategy`.
if (this._ngElementStrategy) {
this._ngElementStrategy.disconnect();
}
if (this.ngElementEventsSubscription) {
@ -186,18 +191,8 @@ export function createCustomElement<P>(
// contain property inputs, use all inputs by default.
inputs.map(({propName}) => propName).forEach(property => {
Object.defineProperty(NgElementImpl.prototype, property, {
get: function() {
if (!this.ngElementStrategy) {
this.ngElementStrategy = strategyFactory.create(config.injector);
}
return this.ngElementStrategy.getInputValue(property);
},
set: function(newValue: any) {
if (!this.ngElementStrategy) {
this.ngElementStrategy = strategyFactory.create(config.injector);
}
this.ngElementStrategy.setInputValue(property, newValue);
},
get: function() { return this.ngElementStrategy.getInputValue(property); },
set: function(newValue: any) { this.ngElementStrategy.setInputValue(property, newValue); },
configurable: true,
enumerable: true,
});