Skip to content
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

Flatten styles array #437

Merged
merged 2 commits into from
Jan 17, 2019
Merged

Flatten styles array #437

merged 2 commits into from
Jan 17, 2019

Conversation

LarsDenBakker
Copy link
Contributor

@LarsDenBakker LarsDenBakker commented Jan 11, 2019

Fixes #429

Flattens styles array before deduplication.

Recursive types are a bit funky, I had to use an interface as instructed here: https://github.com/unional/typescript-guidelines/blob/master/pages/advance-types/recursive-types.md

I'm using Array.prototype.flat when supported, otherwise there is a minimal implementation.

I had to enable esnext typings for Array.prototype.flat support.

@justinfagnani
Copy link
Contributor

See my comments on #429

@@ -22,6 +22,28 @@ export {html, svg, TemplateResult, SVGTemplateResult} from 'lit-html/lit-html';
import {supportsAdoptingStyleSheets, CSSResult} from './lib/css-tag.js';
export * from './lib/css-tag.js';

export interface LitElementStyles extends Array<CSSResult | CSSResult[] | LitElementStyles> {}
Copy link
Member

@sorvell sorvell Jan 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also address #429 (comment) by allowing this to be a single CSSResult. This way static get styles can be a single CSSResult and then any form of composition (sharing some "style modules" or some super class styles) is similar, e.g. static get styles() { return [super.styles, sharedStyles]; }.

If we do this, then flattenStyles needs to handle the case where the input is a non-array and we'll obviously need to test that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion, added it in :)

@sorvell sorvell merged commit 18b566e into lit:master Jan 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants