Skip to content

Commit

Permalink
Fix noted TODOs (#2072)
Browse files Browse the repository at this point in the history
* Fixes doc link TODOs in LitElement and ReactiveElement
* Fixes styling tests to include static bindings in lit-html and LitElement
* Removes `@queryAssignedNodes` shim of `Element.matches` since this is now addressed via polyfills.
  • Loading branch information
Steve Orvell committed Aug 27, 2021
1 parent d6b385e commit 7adfbb0
Show file tree
Hide file tree
Showing 13 changed files with 29 additions and 46 deletions.
7 changes: 7 additions & 0 deletions .changeset/heavy-melons-walk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'lit-element': patch
'lit-html': patch
'@lit/reactive-element': patch
---

Remove unneeded `matches` support in @queryAssignedNodes. Update styling tests to use static bindings where needed. Fix TODOs related to doc links.
1 change: 0 additions & 1 deletion packages/internal-scripts/package-lock.json

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

1 change: 0 additions & 1 deletion packages/lit-element/package-lock.json

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

3 changes: 1 addition & 2 deletions packages/lit-element/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ export * from 'lit-html';
export * from './lit-element.js';
export * from './decorators.js';

// TODO: link to docs on the new site
console.warn(
"The main 'lit-element' module entrypoint is deprecated. Please update " +
"your imports to use the 'lit' package: 'lit' and 'lit/decorators.ts' " +
"or import from 'lit-element/lit-element.ts'."
"or import from 'lit-element/lit-element.ts'. See https://lit.dev/docs/releases/upgrade/#update-packages-and-import-paths for more information."
);
6 changes: 3 additions & 3 deletions packages/lit-element/src/lit-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ if (DEV_MODE) {
if (obj[name] !== undefined) {
console.warn(
`\`${name}\` is implemented. It ` +
`has been removed from this version of LitElement. `
// TODO(sorvell): add link to changelog when location has stabilized.
// + See the changelog at https://github.com/lit/lit/blob/main/packages/lit-element/CHANGELOG.md`
`has been removed from this version of LitElement. See ` +
`https://lit.dev/docs/releases/upgrade/#litelement ` +
`for more information.`
);
}
};
Expand Down
12 changes: 6 additions & 6 deletions packages/lit-element/src/test/lit-element_styling_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* SPDX-License-Identifier: BSD-3-Clause
*/
import {css, html as htmlWithStyles, LitElement} from '../lit-element.js';
import {html as staticHtml, unsafeStatic} from 'lit-html/static.js';

import {
canTestLitElement,
Expand Down Expand Up @@ -334,19 +335,17 @@ import {assert} from '@esm-bundle/chai';
}
});

// TODO(sorvell): Bindings in styles are no longer supported.
// This will be supported via static bindings only.
test.skip('properties in styles render with initial value and cannot be changed', async () => {
test('static properties in styles render with initial value and cannot be changed', async () => {
let border = `6px solid blue`;
const name = generateElementName();
customElements.define(
name,
class extends LitElement {
override render() {
return htmlWithStyles`
return staticHtml`
<style>
div {
border: ${border};
border: ${unsafeStatic(border)};
}
</style>
<div>Testing...</div>`;
Expand All @@ -356,14 +355,15 @@ import {assert} from '@esm-bundle/chai';
const el = document.createElement(name) as LitElement;
container.appendChild(el);
await el.updateComplete;
const div = el.shadowRoot!.querySelector('div');
let div = el.shadowRoot!.querySelector('div');
assert.equal(
getComputedStyleValue(div!, 'border-top-width').trim(),
'6px'
);
border = `4px solid orange`;
el.requestUpdate();
await el.updateComplete;
div = el.shadowRoot!.querySelector('div');
assert.equal(
getComputedStyleValue(div!, 'border-top-width').trim(),
'6px'
Expand Down
1 change: 0 additions & 1 deletion packages/lit-html/package-lock.json

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

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {repeat} from '../../directives/repeat.js';
import {cache} from '../../directives/cache.js';
import {assert} from '@esm-bundle/chai';
import {renderShadowRoot, wrap, shadowRoot} from '../test-utils/shadow-root.js';
import {html as staticHtml, unsafeStatic} from '../../static.js';

import '../lit-html_test.js';
// selected directive tests
Expand Down Expand Up @@ -387,32 +388,32 @@ suite('polyfill-support rendering', () => {
wrap(document.body).removeChild(container2);
});

// TODO(sorvell): This will only be supported via static bindings.
test.skip('part values render into styles once per scope', function () {
test('static part values render into styles once per scope', function () {
if (typeof window.ShadyDOM === 'undefined' || !window.ShadyDOM.inUse) {
this.skip();
return;
}
const container = document.createElement('scope-3');
wrap(document.body).appendChild(container);
const renderTemplate = (border: string) => {
const result = html`
const result = staticHtml`
<style>
div {
border: ${border};
border: ${unsafeStatic(border)};
}
</style>
<div>Testing...</div>
`;
renderShadowRoot(result, container);
};
renderTemplate('1px solid black');
const div = shadowRoot(container)!.querySelector('div');
let div = shadowRoot(container)!.querySelector('div');
assert.equal(
getComputedStyle(div!).getPropertyValue('border-top-width').trim(),
'1px'
);
renderTemplate('2px solid black');
div = shadowRoot(container)!.querySelector('div');
assert.equal(
getComputedStyle(div!).getPropertyValue('border-top-width').trim(),
'1px'
Expand Down
1 change: 0 additions & 1 deletion packages/lit/package-lock.json

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

12 changes: 1 addition & 11 deletions packages/reactive-element/src/decorators/query-assigned-nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@
import {ReactiveElement} from '../reactive-element.js';
import {decorateProperty} from './base.js';

// TODO(sorvell): Remove when https://github.com/webcomponents/polyfills/issues/397 is addressed.
// x-browser support for matches
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const ElementProto = Element.prototype as any;
const legacyMatches =
ElementProto.msMatchesSelector || ElementProto.webkitMatchesSelector;

/**
* A property decorator that converts a class property into a getter that
* returns the `assignedNodes` of the given named `slot`. Note, the type of
Expand Down Expand Up @@ -64,10 +57,7 @@ export function queryAssignedNodes(
nodes = nodes.filter(
(node) =>
node.nodeType === Node.ELEMENT_NODE &&
// eslint-disable-next-line @typescript-eslint/no-explicit-any
((node as any).matches
? (node as Element).matches(selector)
: legacyMatches.call(node as Element, selector))
(node as Element).matches(selector)
);
}
return nodes;
Expand Down
6 changes: 4 additions & 2 deletions packages/reactive-element/src/reactive-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1074,7 +1074,6 @@ export abstract class ReactiveElement
}
);
if (shadowedProperties.length) {
// TODO(sorvell): Link to docs explanation of this issue.
console.warn(
`The following properties will not trigger updates as expected ` +
`because they are set using class fields: ` +
Expand All @@ -1083,7 +1082,10 @@ export abstract class ReactiveElement
`accessors used for detecting changes. To fix this issue, ` +
`either initialize properties in the constructor or adjust ` +
`your compiler settings; for example, for TypeScript set ` +
`\`useDefineForClassFields: false\` in your \`tsconfig.json\`.`
`\`useDefineForClassFields: false\` in your \`tsconfig.json\`.` +
`See https://lit.dev/docs/components/properties/#declare and ` +
`https://lit.dev/docs/components/decorators/` +
`#avoiding-issues-with-class-fields for more information.`
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,15 +198,7 @@ const flush =
assert.deepEqual(el.assignedNodesEl.footerAssignedItems, []);
});

test('returns assignedNodes for slot that contains text nodes filtered by selector when Element.matches does not exist', () => {
const descriptor = Object.getOwnPropertyDescriptor(
Element.prototype,
'matches'
);
Object.defineProperty(Element.prototype, 'matches', {
value: undefined,
configurable: true,
});
test('returns assignedNodes for slot that contains text nodes filtered by selector', () => {
assert.deepEqual(el.assignedNodesEl.footerAssignedItems, []);
const child1 = document.createElement('div');
const child2 = document.createElement('div');
Expand All @@ -222,9 +214,6 @@ const flush =
el.removeChild(child2);
flush();
assert.deepEqual(el.assignedNodesEl.footerAssignedItems, []);
if (descriptor !== undefined) {
Object.defineProperty(Element.prototype, 'matches', descriptor);
}
});

test('always returns an array, even if the slot is not rendered', () => {
Expand Down
1 change: 0 additions & 1 deletion packages/tests/package-lock.json

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

0 comments on commit 7adfbb0

Please sign in to comment.