-
Notifications
You must be signed in to change notification settings - Fork 318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removes default for static get styles
#456
Conversation
Fixes #452. In addition... * Fixes type settings so `Array.flat` works. * Optimization: ensures `this.styles` is accessed only once per definition. Since this getter generates CSSResults, it should not be run superfluously.
src/lit-element.ts
Outdated
@@ -66,14 +66,15 @@ export class LitElement extends UpdatingElement { | |||
* Array of styles to apply to the element. The styles should be defined | |||
* using the `css` tag function. | |||
*/ | |||
static get styles(): CSSResult | CSSResultArray { return []; } | |||
static styles?: CSSResult | CSSResultArray; | |||
|
|||
private static _styles: CSSResult[]|undefined; | |||
|
|||
private static get _uniqueStyles(): CSSResult[] { | |||
if (this._styles === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs to be hasOwnProperty
for subclassing to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to use a own property check on styles
too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
src/lit-element.ts
Outdated
@@ -66,14 +66,15 @@ export class LitElement extends UpdatingElement { | |||
* Array of styles to apply to the element. The styles should be defined | |||
* using the `css` tag function. | |||
*/ | |||
static get styles(): CSSResult | CSSResultArray { return []; } | |||
static styles?: CSSResult | CSSResultArray; | |||
|
|||
private static _styles: CSSResult[]|undefined; | |||
|
|||
private static get _uniqueStyles(): CSSResult[] { | |||
if (this._styles === undefined) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to use a own property check on styles
too
src/lit-element.ts
Outdated
const styles = flattenStyles(this.styles); | ||
const userStyles = this.styles; | ||
if (Array.isArray(userStyles)) { | ||
const styles = flattenStyles(userStyles); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider not re-flattening if !this.hasOwnProperty('styles')
Also add TODO to cache CSSResult
s to avoid creating new stylesheet objects for subclasses that spread super.styles
into this.styles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the changelog too
Fixes #452.
In addition...
Array.flat
works.this.styles
is accessed only once per definition. Since this getter generates CSSResults, it should not be run superfluously.