Implements nesting in template string parser #1103
Conversation
|
Also cc @eps1lon |
| @@ -44,7 +44,7 @@ module.exports = config => { | |||
| frameworks: ['benchmark'], | |||
| // Using a fixed position for a file name, m.b. should use an args parser later. | |||
| files: [process.argv[4] || 'packages/jss/benchmark/**/*.js'], | |||
| preprocessors: {'packages/jss/benchmark/**/*.js': ['webpack']}, | |||
| preprocessors: {'packages/**/benchmark/**/*.js': ['webpack']}, | |||
| @@ -4,7 +4,7 @@ import template from '../../src/index' | |||
| import parse from '../../src/parse' | |||
|
|
|||
| const options = {Renderer: null} | |||
| const jss = create(options).use(template()) | |||
| const jss = create(options).use(template({cache: false})) | |||
There was a problem hiding this comment.
disabling cache here to have a realistic comparison without caching, I didn't document it, because I don't think user should need it, so for now it's a private one, just for tests
package.json
Outdated
| @@ -15,7 +15,7 @@ | |||
| "format:ci": "yarn format --list-different", | |||
| "test": "karma start --single-run", | |||
| "test:watch": "karma start", | |||
| "test:bench": "cross-env BENCHMARK=true karma start --single-run", | |||
| "test:bench": "cross-env NODE_ENV=production BENCHMARK=true karma start --single-run", | |||
There was a problem hiding this comment.
removes dead from the bench~!
| suite('Parse', () => { | ||
| benchmark('parse()', () => { | ||
| parse(css) | ||
| }) | ||
|
|
||
| benchmark('stylis()', () => { |
There was a problem hiding this comment.
using stylis parser just to see a comparable other value in perspective
| if (openCurlyIndex === -1) { | ||
| warning(false, `[JSS] Missing opening curly brace in "${decl}".`) | ||
| } | ||
| const key = decl.substring(0, openCurlyIndex - 1) |
There was a problem hiding this comment.
Should we trim the selector here?
| } | ||
| const key = decl.substring(0, openCurlyIndex - 1) | ||
| const nestedStyle = {} | ||
| rules[rules.length - 1][key] = nestedStyle |
There was a problem hiding this comment.
Why do we add the key here to the last object?
There was a problem hiding this comment.
rules array represents an array of nested styles, each level, separate array entry, so we need to do both, reference the nested style in the parent object and add it to the array to have a pointer to the current style object where declarations will be added next in that pass
|
|
||
| // We are closing a nested rule. | ||
| if (decl === '}') { | ||
| rules.pop() |
There was a problem hiding this comment.
Shouldn't we add the rule when we pop it to the style?
There was a problem hiding this comment.
we already did, we are just removing it from the array, since we basically finished adding declarations to that rule
|
@kof should we merge this? |
Implements #837
I think I found a way to implement it without degrading performance, LOC is still pretty small.
The problem it is solving: you can't express a nested rule with a template string. If you used a template string initially to describe the rule, as soon as you want to add nesting, you will have to rewrite it to objects.