Skip to content

Commit

Permalink
Merge pull request #181 from Polymer/native-property-limitation
Browse files Browse the repository at this point in the history
Address native properties issue on older browsers
  • Loading branch information
Steve Orvell authored Sep 12, 2018
2 parents b4c5f51 + 91b4078 commit fafb487
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 48 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,8 @@ Chrome, Safari, Opera, Firefox, Edge. In addition, Internet Explorer 11 is also

## Known Issues

* On very old versions of Safari (<=9) or Chrome (<=41), properties created for native
platform properties like (`id` or `name`) may not have default values set in the element constructor.
On these browsers native properties appear on instances and therefore their default value
will overwrite any element default (e.g. if the element sets this.id = 'id' in the constructor,
the 'id' will become '' since this is the native platform default).
54 changes: 27 additions & 27 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,23 @@
"devDependencies": {
"@types/chai": "^4.0.1",
"@types/mocha": "^5.2.4",
"@webcomponents/webcomponentsjs": "^2.1.0",
"@webcomponents/webcomponentsjs": "^2.1.1",
"chai": "^4.0.2",
"mocha": "^5.0.5",
"rollup": "^0.64.1",
"rollup-plugin-filesize": "^4.0.1",
"rollup-plugin-node-resolve": "^3.3.0",
"rollup-plugin-node-resolve": "^3.4.0",
"rollup-plugin-terser": "^1.0.1",
"tslint": "^5.7.0",
"typedoc": "^0.8.0",
"typescript": "^3.0.1",
"typescript": "^3.0.3",
"uglify-es": "^3.3.9",
"wct-browser-legacy": "^1.0.1",
"web-component-tester": "^6.8.0"
},
"typings": "lit-element.d.ts",
"dependencies": {
"lit-html": "^0.11.0"
"lit-html": "^0.11.2"
},
"publishConfig": {
"access": "public"
Expand Down
7 changes: 6 additions & 1 deletion src/lib/updating-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,12 @@ export abstract class UpdatingElement extends HTMLElement {
* Fixes any properties set on the instance before upgrade time.
* Otherwise these would shadow the accessor and break these properties.
* The properties are stored in a Map which is played back after the constructor
* runs.
* runs. Note, on very old versions of Safari (<=9) or Chrome (<=41), properties
* created for native platform properties like (`id` or `name`) may not have default
* values set in the element constructor. On these browsers native properties
* appear on instances and therefore their default value will overwrite any element
* default (e.g. if the element sets this.id = 'id' in the constructor, the 'id'
* will become '' since this is the native platform default).
*/
private _saveInstanceProperties() {
for (const [p] of (this.constructor as typeof UpdatingElement)._classProperties) {
Expand Down
25 changes: 9 additions & 16 deletions src/test/lit-element_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1200,6 +1200,8 @@ suite('LitElement', () => {
assert.equal(el.getAttribute('zot'), '3');
});

// Note, on older browsers (e.g. old Safari/Chrome), native properties
// cannot have default values. These will be overwritten by instance values.
test('can make properties for native accessors', async () => {
class E extends LitElement {

Expand All @@ -1212,19 +1214,11 @@ suite('LitElement', () => {
};
}

name: string;
foo: string;
name: string|undefined;
foo = '';

changedProperties: PropertyValues|undefined = undefined;

constructor() {
super();
this.id = 'id';
this.name = 'name';
this.title = 'title';
this.foo = 'foo';
}

update(changedProperties: PropertyValues) {
(this as any).zot = (this as any).foo + (this as any).bar;
super.update(changedProperties);
Expand All @@ -1240,12 +1234,11 @@ suite('LitElement', () => {
const el = new E() as any;
container.appendChild(el);
await el.updateComplete;
const testMap = new Map();
testMap.set('id', undefined);
testMap.set('name', undefined);
testMap.set('title', undefined);
testMap.set('foo', undefined);
assert.deepEqual(el.changedProperties, testMap);
el.foo = 'foo';
el.id = 'id';
el.name = 'name';
el.title = 'title';
await el.updateComplete;
assert.equal(el.shadowRoot!.textContent, 'id-name-title-foo');
assert.equal((window as any).id, el);
el.id = 'id2';
Expand Down

0 comments on commit fafb487

Please sign in to comment.