feat: css at_value support #18535
feat: css at_value support #18535
Conversation
|
For maintainers only:
|
|
Let's add more tests like I written in slack |
faga295
force-pushed
the
feat/css_atvalue_support
branch
from
6b83bd2
to
1ee76dc
Compare
lib/css/CssParser.js
Outdated
| pos = walkCssTokens.eatWhitespaceAndComments(input, pos); | ||
| if (pos === input.length) return pos; | ||
| let value; | ||
| [pos, value] = eatText(input, pos, eatUntil(":{};/")); |
There was a problem hiding this comment.
I think it should be only ; here? Do we really need parse it? Maybe we can parse eveyrything to ;/from (future improvement)?
There was a problem hiding this comment.
if only eat until ;, then how to solve the situation where user wirtes unexpected cahracter or the user forgets to write ';'
There was a problem hiding this comment.
Do you mean eat until ";" and collect the string between @value and ";" and then use a regex to match to key and value like postcss-modules-values, this would be more flexible, but maybe a little inappropriate
faga295
force-pushed
the
feat/css_atvalue_support
branch
from
1a0053b
to
6507aea
Compare
| --color1: v-empty; | ||
| --color2: v-empty-1; | ||
| --color3: v-empty-2; | ||
| } |
There was a problem hiding this comment.
There are more tests here https://github.com/css-modules/postcss-modules-values/blob/master/test/index.test.js, please add all of them
lib/css/CssParser.js
Outdated
| @@ -658,6 713,11 @@ class CssParser extends Parser { | |||
| modeData = isLocalMode() ? "local" : "global"; | |||
| isNextRulePrelude = true; | |||
| return end; | |||
| } else if (name === "@value") { | |||
| const pos = parseModulesValues(input, end); | |||
| const dep = new ConstDependency("", [start, pos]); | |||
There was a problem hiding this comment.
We need to implement other dep, because @value can have url - @value blue, red from "./colors.css
There was a problem hiding this comment.
I want to implement @value import using :import, like css loader
There was a problem hiding this comment.
Maybe I should implement :import firstly
There was a problem hiding this comment.
I think they will use different dependecies, because @value -> :import {} -> dependecy is not good for perf
There was a problem hiding this comment.
We can separate @value into value definition and value imports, similar to postModulesValues. This PR currently only implements value definition, which can be handled using ConstDependency. value imports can be managed through a dependency, perhaps called CssIcssImportDependency, which deals with the :import functionality.
lib/css/CssParser.js
Outdated
| @@ -821,6 881,13 @@ class CssParser extends Parser { | |||
| identifier: (input, start, end) => { | |||
| switch (scope) { | |||
| case CSS_MODE_IN_BLOCK: { | |||
| const item = modulesValues.find( | |||
| item => input.slice(start, end) === item.key | |||
| ); | |||
There was a problem hiding this comment.
You can use Map (or WeakMap with Map using parser as a key for weak map and input.slice(start, end) as a key for Map), it will be very fast and effective
faga295
force-pushed
the
feat/css_atvalue_support
branch
from
d98332d
to
9cb3e97
Compare
faga295
force-pushed
the
feat/css_atvalue_support
branch
from
52baef3
to
41ec556
Compare
|
Done here - #18945 |
#14893 after
icss import#18568 is supported,@valueimport can continueWhat kind of change does this PR introduce?
feature
Did you add tests for your changes?
Yes
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
Nothing