fix(elements): capture input properties set before upgrading the element (#36114)
Previously, if an element started out as a regular `HTMLElement` (not a Custom Element) and was later upgraded to a Custom Element, any properties corresponding to component inputs that were set on the element before upgrading it would not be captured correctly and thus not reflected on the instantiated component. This commit fixes it by ensuring that such properties are captured correctly. Jira issue: [FW-2006](https://angular-team.atlassian.net/browse/FW-2006) Fixes #30848 Closes #31416 PR Close #36114
This commit is contained in:
George Kalpakas
Kara Erickson
parent
cb719ee16e
commit
2fc5ae561b
|
|
@ -145,9 +145,32 @@ export function createCustomElement<P>(
|
|||
// 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.
|
||||
if (this._ngElementStrategy === null) {
|
||||
this._ngElementStrategy = strategyFactory.create(this.injector);
|
||||
const strategy = this._ngElementStrategy = strategyFactory.create(this.injector);
|
||||
|
||||
// Collect pre-existing values on the element to re-apply through the strategy.
|
||||
const preExistingValues =
|
||||
inputs.filter(({propName}) => this.hasOwnProperty(propName)).map(({propName}): [
|
||||
string, any
|
||||
] => [propName, (this as any)[propName]]);
|
||||
|
||||
// In some browsers (e.g. IE10), `Object.setPrototypeOf()` (which is required by some Custom
|
||||
// Elements polyfills) is not defined and is thus polyfilled in a way that does not preserve
|
||||
// the prototype chain. In such cases, `this` will not be an instance of `NgElementImpl` and
|
||||
// thus not have the component input getters/setters defined on `NgElementImpl.prototype`.
|
||||
if (!(this instanceof NgElementImpl)) {
|
||||
// Add getters and setters to the instance itself for each property input.
|
||||
defineInputGettersSetters(inputs, this);
|
||||
} else {
|
||||
// Delete the property from the instance, so that it can go through the getters/setters
|
||||
// set on `NgElementImpl.prototype`.
|
||||
preExistingValues.forEach(([propName]) => delete (this as any)[propName]);
|
||||
}
|
||||
return this._ngElementStrategy;
|
||||
|
||||
// Re-apply pre-existing values through the strategy.
|
||||
preExistingValues.forEach(([propName, value]) => strategy.setInputValue(propName, value));
|
||||
}
|
||||
|
||||
return this._ngElementStrategy!;
|
||||
}
|
||||
|
||||
private readonly injector: Injector;
|
||||
|
|
@ -193,16 +216,26 @@ export function createCustomElement<P>(
|
|||
// Update the property descriptor of `NgElementImpl#ngElementStrategy` to make it enumerable.
|
||||
Object.defineProperty(NgElementImpl.prototype, 'ngElementStrategy', {enumerable: true});
|
||||
|
||||
// Add getters and setters to the prototype for each property input. If the config does not
|
||||
// contain property inputs, use all inputs by default.
|
||||
inputs.map(({propName}) => propName).forEach(property => {
|
||||
Object.defineProperty(NgElementImpl.prototype, property, {
|
||||
get: function() { return this.ngElementStrategy.getInputValue(property); },
|
||||
set: function(newValue: any) { this.ngElementStrategy.setInputValue(property, newValue); },
|
||||
// Add getters and setters to the prototype for each property input.
|
||||
defineInputGettersSetters(inputs, NgElementImpl.prototype);
|
||||
|
||||
return (NgElementImpl as any) as NgElementConstructor<P>;
|
||||
}
|
||||
|
||||
// Helpers
|
||||
function defineInputGettersSetters(
|
||||
inputs: {propName: string, templateName: string}[], target: object): void {
|
||||
// Add getters and setters for each property input.
|
||||
inputs.forEach(({propName}) => {
|
||||
Object.defineProperty(target, propName, {
|
||||
get(): any {
|
||||
return this.ngElementStrategy.getInputValue(propName);
|
||||
},
|
||||
set(newValue: any): void {
|
||||
this.ngElementStrategy.setInputValue(propName, newValue);
|
||||
},
|
||||
configurable: true,
|
||||
enumerable: true,
|
||||
});
|
||||
});
|
||||
|
||||
return (NgElementImpl as any) as NgElementConstructor<P>;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -22,12 +22,16 @@ type WithFooBar = {
|
|||
|
||||
if (browserDetection.supportsCustomElements) {
|
||||
describe('createCustomElement', () => {
|
||||
let selectorUid = 0;
|
||||
let testContainer: HTMLDivElement;
|
||||
let NgElementCtor: NgElementConstructor<WithFooBar>;
|
||||
let strategy: TestStrategy;
|
||||
let strategyFactory: TestStrategyFactory;
|
||||
let injector: Injector;
|
||||
|
||||
beforeAll(done => {
|
||||
testContainer = document.createElement('div');
|
||||
document.body.appendChild(testContainer);
|
||||
destroyPlatform();
|
||||
platformBrowserDynamic()
|
||||
.bootstrapModule(TestModule)
|
||||
|
|
@ -36,18 +40,23 @@ if (browserDetection.supportsCustomElements) {
|
|||
strategyFactory = new TestStrategyFactory();
|
||||
strategy = strategyFactory.testStrategy;
|
||||
|
||||
NgElementCtor = createCustomElement(TestComponent, {injector, strategyFactory});
|
||||
const {selector, ElementCtor} = createTestCustomElement();
|
||||
NgElementCtor = ElementCtor;
|
||||
|
||||
// The `@webcomponents/custom-elements/src/native-shim.js` polyfill allows us to create
|
||||
// new instances of the NgElement which extends HTMLElement, as long as we define it.
|
||||
customElements.define('test-element', NgElementCtor);
|
||||
customElements.define(selector, NgElementCtor);
|
||||
})
|
||||
.then(done, done.fail);
|
||||
});
|
||||
|
||||
afterEach(() => strategy.reset());
|
||||
|
||||
afterAll(() => destroyPlatform());
|
||||
afterAll(() => {
|
||||
destroyPlatform();
|
||||
document.body.removeChild(testContainer);
|
||||
(testContainer as any) = null;
|
||||
});
|
||||
|
||||
it('should use a default strategy for converting component inputs', () => {
|
||||
expect(NgElementCtor.observedAttributes).toEqual(['foo-foo', 'barbar']);
|
||||
|
|
@ -113,7 +122,90 @@ if (browserDetection.supportsCustomElements) {
|
|||
expect(strategy.inputs.get('barBar')).toBe('barBar-value');
|
||||
});
|
||||
|
||||
it('should capture properties set before upgrading the element', () => {
|
||||
// Create a regular element and set properties on it.
|
||||
const {selector, ElementCtor} = createTestCustomElement();
|
||||
const element = Object.assign(document.createElement(selector), {
|
||||
fooFoo: 'foo-prop-value',
|
||||
barBar: 'bar-prop-value',
|
||||
});
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-prop-value');
|
||||
|
||||
// Upgrade the element to a Custom Element and insert it into the DOM.
|
||||
customElements.define(selector, ElementCtor);
|
||||
testContainer.appendChild(element);
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-prop-value');
|
||||
|
||||
expect(strategy.inputs.get('fooFoo')).toBe('foo-prop-value');
|
||||
expect(strategy.inputs.get('barBar')).toBe('bar-prop-value');
|
||||
});
|
||||
|
||||
it('should capture properties set after upgrading the element but before inserting it into the DOM',
|
||||
() => {
|
||||
// Create a regular element and set properties on it.
|
||||
const {selector, ElementCtor} = createTestCustomElement();
|
||||
const element = Object.assign(document.createElement(selector), {
|
||||
fooFoo: 'foo-prop-value',
|
||||
barBar: 'bar-prop-value',
|
||||
});
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-prop-value');
|
||||
|
||||
// Upgrade the element to a Custom Element (without inserting it into the DOM) and update a
|
||||
// property.
|
||||
customElements.define(selector, ElementCtor);
|
||||
customElements.upgrade(element);
|
||||
element.barBar = 'bar-prop-value-2';
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-prop-value-2');
|
||||
|
||||
// Insert the element into the DOM.
|
||||
testContainer.appendChild(element);
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-prop-value-2');
|
||||
|
||||
expect(strategy.inputs.get('fooFoo')).toBe('foo-prop-value');
|
||||
expect(strategy.inputs.get('barBar')).toBe('bar-prop-value-2');
|
||||
});
|
||||
|
||||
it('should allow overwriting properties with attributes after upgrading the element but before inserting it into the DOM',
|
||||
() => {
|
||||
// Create a regular element and set properties on it.
|
||||
const {selector, ElementCtor} = createTestCustomElement();
|
||||
const element = Object.assign(document.createElement(selector), {
|
||||
fooFoo: 'foo-prop-value',
|
||||
barBar: 'bar-prop-value',
|
||||
});
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-prop-value');
|
||||
|
||||
// Upgrade the element to a Custom Element (without inserting it into the DOM) and set an
|
||||
// attribute.
|
||||
customElements.define(selector, ElementCtor);
|
||||
customElements.upgrade(element);
|
||||
element.setAttribute('barbar', 'bar-attr-value');
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-attr-value');
|
||||
|
||||
// Insert the element into the DOM.
|
||||
testContainer.appendChild(element);
|
||||
expect(element.fooFoo).toBe('foo-prop-value');
|
||||
expect(element.barBar).toBe('bar-attr-value');
|
||||
|
||||
expect(strategy.inputs.get('fooFoo')).toBe('foo-prop-value');
|
||||
expect(strategy.inputs.get('barBar')).toBe('bar-attr-value');
|
||||
});
|
||||
|
||||
// Helpers
|
||||
function createTestCustomElement() {
|
||||
return {
|
||||
selector: `test-element-${++selectorUid}`,
|
||||
ElementCtor: createCustomElement<WithFooBar>(TestComponent, {injector, strategyFactory}),
|
||||
};
|
||||
}
|
||||
|
||||
@Component({
|
||||
selector: 'test-component',
|
||||
template: 'TestComponent|foo({{ fooFoo }})|bar({{ barBar }})',
|
||||
|
|
|
|||
Loading…
Reference in New Issue