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

feat(css-modules): added multiple modules support; re-write tests & add more css module scenarios #2045

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RaphaelDDL
Copy link

@RaphaelDDL RaphaelDDL commented May 18, 2023

Hello,

The issue was first posted in CSS-Loader webpack-contrib/css-loader#1518 as I wasn't sure if was a css-loader issue or a vue-loader issue. After very helpful assistance from Alex and upon further investigation, the issue lies in how vue-loader imports each file's style tags.

You can read my post in the link above, and also see my PoC for various test cases https://github.com/RaphaelDDL/vue3-css-module (and ofcourse, the PR where I added most of the cases as fixtures + tests)

To try sum up, in vue-loader documentation there's an entry for CSS Module regarding Custom Inject Name:

You can have more than one <style> tags in a single *.vue component. To avoid injected styles to overwrite each other, you can customize the name of the injected computed property by giving the module attribute a value:

    <style module="a">
    /* identifiers injected as a */
    </style>

    <style module="b">
    /* identifiers injected as b */
    </style>

This and different does NOT work in a few scenarios. In this specific scenario of a and b, it only works if each module has it's own classes, e.g. module=a has class .a and module=b has class .b. If both contain class .c, instead of having a.c and b.c as two different styles, they both end up with same name, same hash, generate same css and overwrite each other in the browser. Discussing below I'll go through this scenario in the test "multiple named modules (different module names, same class name)".

The module issues

The main gist is that even though styleLoader loops through each style correctly, and giving the data correctly to genCSSModulesCode, each loop will overwrite the last css module class hashmap depending on the setup. This way, only the LAST style actually contains it's classes added to $style (or your name of choice for named module="something").

Upon further investigation, unfortunately there's no test which would highlight these issues. The current test contains one scenario which is (named)+(nameless scoped) (file tested: https://github.com/vuejs/vue-loader/blob/main/test/fixtures/css-modules.vue and test itself: https://github.com/vuejs/vue-loader/blob/main/test/style.spec.ts#L127-L151 )

With my new suite of test scenarios, here's the current test results with no code changes:

  CSS Modules
    ✓ default/nameless module ($style) (520 ms)
    ✓ named module (module="named") (401 ms)
    ✕ multiple default/multiple modules (210 ms)
    ✕ multiple named modules (same module names, different class names) (183 ms)
    ✓ multiple named modules (same module names, same class names) (183 ms)
    ✓ multiple named modules (different module names, different class names) (382 ms)
    ✕ multiple named modules (different module names, same class name) (185 ms)
    ✕ default/nameless extend (same class name) (191 ms)
    ✕ default/nameless extend (different classes) (183 ms)
    ✓ default/nameless extend w/ compose importing css file (348 ms)

Here's with my fix in cssModules.ts:

  CSS Modules
    ✓ default/nameless module ($style) (478 ms)
    ✓ named module (module="named") (400 ms)
    ✓ multiple default/multiple modules (386 ms)
    ✓ multiple named modules (same module names, different class names) (376 ms)
    ✓ multiple named modules (same module names, same class names) (183 ms)
    ✓ multiple named modules (different module names, different class names) (368 ms)
    ✕ multiple named modules (different module names, same class name) (178 ms)
    ✕ default/nameless extend (same class name) (174 ms)
    ✕ default/nameless extend (different classes) (187 ms)
    ✓ default/nameless extend w/ compose importing css file (350 ms)

Pending issues

You'll see there are 3 cases which still are broken. So of course, the CI run will fail, as those 3 fail LOL. That's because I'm not entirely sure on where to fix those scenarios and I would love to get your input on how to fix those as well.

I left the comment in each test, and I'll discuss here as well:

✕ multiple named modules (different module names, same class name) (178 ms)

In this scenario, as mentioned prior, classes created have same name+hash as hash not taking module name into account, therefore by having same class names in different modules, each module gets same hash. According to Alex in the css-loader, vue-loader needs to pass it to css-loader too for the hash to be properly done. He mention something regarding vue-loader not using css-loader's specific !=! syntax for that.

An example on how the request path is built with current code:

"./node_modules/css-loader/dist/cjs.js??clonedRuleSet-3.use[1]!./node_modules/vue-loader/dist/stylePostLoader.js!./node_modules/postcss-loader/dist/cjs.js??clonedRuleSet-3.use[2]!./node_modules/sass-loader/dist/cjs.js??clonedRuleSet-3.use[3]!./node_modules/vue-loader/dist/index.js??ruleSet[1].rules[6].use[0]!./src/components/module-named-multiple.vue?vue&type=style&index=0&id=18f2a545&lang=scss&module=moduledefault":

✕ default/nameless extend (same class name) (174 ms)

extend is adding both classes to style tags in document but instance.$style.red className points to the file's original style (hexcolor)
(a677feb62ef42886b712f1b16b71e851-vue) and not the extended version (red keyword)
e.g.

._7ef3af38102f7bc2284518b4f9dda8d9-vue {
    color: red;
}
.a677feb62ef42886b712f1b16b71e851-vue {
    color: #FF0000;
}

I would expect that when extending with same class names, the extension would overwrite? Or my assumption is incorrect?

✕ default/nameless extend (different classes) (187 ms)

Styles for both own file's style tag and the extended file are being added to style tags in document BUT instance.$style has not received the red in the hashmap so instance.$style.red does't exist

//=== instance.$style
{ black: "dd07afd7f1529b35227b9b3bc7609e28-vue" }


//=== styles
._7ef3af38102f7bc2284518b4f9dda8d9-vue {
    color: red;
}


.dd07afd7f1529b35227b9b3bc7609e28-vue {
    color: #000000;
}

I am also not sure if my assumption is correct, but extending using extends from anoyher vue I would expect that the extended vue's styles would be now available in the locals/$style.

Extra changes

I also included a few fixes which were present in the original CSS Module tests and that I also used in my new ones:

  • The RegExp \w{5} for the custom localIdentName which includes a short hash [hash:base64:5] is incomplete. The hash can also include - and +, which are not included in the \w keyword, e.g. Expected: /^red-\w{5}, received: red_-euaf and red--pnf+t
  • Same of above applies to the "default" identName, /^\w{21,}/ and was fixed to /^[\w-+]{21,}/
  • When retrieving the style contents with textContent, some styles are being scaped, specially the ones with = in the hash. Example: Expected class name: .AB2VVFkhhj-I4oTbd0Gcsw==, received: .AB2VVFkhhj-I4oTbd0Gcsw\\=\\=. So I added a normalizeEscapedHash in utils in order to match classNames with style contents correctly in tests.

--

The reasoning behind having more than one module is explained in the css-loader issue, though even without our specific extra brand attribute, the module as is does have issues, as the test suite shows using the vue-loader's docs example as prior mentioned.

I hope you consider evaluating my changes and help with the 3 remaining tests.
I'm available for any questions.

Thank you

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.

None yet

1 participant