Skip to content

Insertion plugin with tree#386

Merged
tkh44 merged 9 commits into
masterfrom
insertion-plugin-with-tree
Oct 6, 2017
Merged

Insertion plugin with tree#386
tkh44 merged 9 commits into
masterfrom
insertion-plugin-with-tree

Conversation

@tkh44
Copy link
Copy Markdown
Member

@tkh44 tkh44 commented Oct 6, 2017

What:
Stylis processes styles from the inside out so we need to build some kind of structure to ensure proper insertion order of the rules.

See #384 for more details.

Why:
Nested pseudo selectors were breaking.

How:
Simple tree structure is used. The idea came from the stylis AST plugin.

Checklist:

  • Documentation N/A
  • Tests
  • Code complete

closes #384

@tkh44 tkh44 requested a review from emmatown October 6, 2017 21:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 6, 2017

Codecov Report

Merging #386 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted Files Coverage Δ
packages/emotion/src/index.js 100% <100%> (ø) ⬆️

Comment thread packages/emotion/src/index.js Outdated
}

function Node(id, selector, parent, ruleText) {
this.id = id
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is id or parent used here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, they were but not anymore.

@tkh44 tkh44 merged commit 83e5377 into master Oct 6, 2017
@tkh44 tkh44 deleted the insertion-plugin-with-tree branch October 6, 2017 22:16
@thysultan
Copy link
Copy Markdown

@tkh44 The current AST plugin in the stylis repo could benefit from a lot of optimizations and improvements. For example this gist demos one that preserves nested children within the AST.

Did you find any pain points that could be used to improve the stylis plugin system? I'm thinking of passing a depth argument to have data about what depth a rule in a nested selector is in.

@emmatown emmatown mentioned this pull request Oct 6, 2017
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

css prop should override styles of styled component

3 participants