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

getCSSModuleLocalIdent may cause unexpected behavior when using moduled CSS class names in querySelector #11775

Open
tychenjiajun opened this issue Dec 16, 2021 · 7 comments

Comments

@tychenjiajun
Copy link

tychenjiajun commented Dec 16, 2021

Describe the bug

Use getLocalIdent: getCSSModuleLocalIdent in css-loader option may cause unexpected behavior when using moduled CSS class names in querySelector.

Did you try recovering your dependencies?

Yes

Which terms did you search for in User Guide?

getCSSModuleLocalIdent

Environment

webpack

Steps to reproduce

(Write your steps here:)

  1. Use exactly the same webpack config in readme.
const getCSSModuleLocalIdent = require('react-dev-utils/getCSSModuleLocalIdent');

// In your webpack config:
// ...
module: {
  rules: [
    {
      test: /\.module\.css$/,
      use: [
        require.resolve('style-loader'),
        {
          loader: require.resolve('css-loader'),
          options: {
            importLoaders: 1,
            modules: {
              getLocalIdent: getCSSModuleLocalIdent,
            },
          },
        },
        {
          loader: require.resolve('postcss-loader'),
          options: postCSSLoaderOptions,
        },
      ],
    },
  ];
}
  1. Write the following codes in app
import styles from '../someStyle.css'

document.querySelector('.' + styles.foo);

function Component() { 
  return <div className={styles.foo} />
}
  1. Hopes for the luck. In some rare cases, styles.foo may includes character + and querySelector will throw error or return null.

Expected behavior

return value of getCSSModuleLocalIdent doesn't includes +.

Actual behavior

return value of getCSSModuleLocalIdent may includes +.

Reproducible demo

Checkout my forks tychenjiajun@28d10e6
In the test cases, if input class name is 'class1020', the output class name will be file_class1020__V+98r. Using this class name inquerySelector causes error, see https://codepen.io/chankcccc/pen/MWEmxpZ

@lithammer
Copy link

We've recently run into this issue as well. It seems like you can work around it by using CSS.escape():

 import styles from '../someStyle.css'

-document.querySelector('.' + styles.foo);
+document.querySelector('.' + CSS.escape(styles.foo));

 function Component() { 
   return <div className={styles.foo} />
 }

@tychenjiajun
Copy link
Author

We've recently run into this issue as well. It seems like you can work around it by using CSS.escape():

 import styles from '../someStyle.css'

-document.querySelector('.' + styles.foo);
+document.querySelector('.' + CSS.escape(styles.foo));

 function Component() { 
   return <div className={styles.foo} />
 }

Good idea. Our solution is copy the original getLocalIndent method and change the hash type from base64 to base62

function getLocalIdent(context, localIdentName, localName, options) {
    // Use the filename or folder name, based on some uses the index.js / index.module.(css|scss|sass) project style
    const fileNameOrFolder = /index\.module\.(css|scss|sass)$/.test(context.resourcePath) ? '[folder]' : '[name]';
    // Create a hash based on a the file location and class name. Will be unique across a project, and close to globally unique.
    const hash = loaderUtils.getHashDigest(path.posix.relative(context.rootContext, context.resourcePath) + localName, 'md5', 'base62', 5);
    // Use loaderUtils to find the file or folder name
    const className = loaderUtils.interpolateName(context, `${fileNameOrFolder}_${localName}__${hash}`, options);
    // Remove the .module that appears in every classname when based on the file and replace all "." with "_".
    return className.replace('.module_', '_').replace(/\./g, '_');
}
// css-loader config
modules: {
    getLocalIdent,
}

Maybe I should submit a PR for it.

@jedlikowski
Copy link

jedlikowski commented Jan 31, 2022

I recently run into that issue as well when the generated classes had / and the browser didn't correctly apply them to elements.
Similar to @tychenjiajun I changed the getLocalIdent function, but instead of using base62, I replaced the 62nd and 63rd characters of base64 with their url-safe counterparts according to https://datatracker.ietf.org/doc/html/rfc4648#section-5

image

This is my code:

// Create a hash based on a the file location and class name. Will be unique across a project, and close to globally unique.
      const hash = loaderUtils
        .getHashDigest(
          path.posix.relative(context.rootContext, context.resourcePath) +
            localName,
          "md5",
          "base64",
          5
        )
        // replace base64 characters with their url-safe counterparts
        // https://datatracker.ietf.org/doc/html/rfc4648#section-5
        .replace(/\+/g, "-")
        .replace(/\//g, "_");

@pstrh
Copy link

pstrh commented Feb 2, 2022

I also run into this issue. My solution is to use "base64url" instead of "base64" as digestType parameter in getHashDigest. base64url was introduced with nodejs 14. As I just raised #12013 regarding css modules hash collisions I can offer to fix the issue there.

BTW I wonder why this issue just appears now.

@jedlikowski
Copy link

jedlikowski commented Feb 2, 2022 via email

@pstrh
Copy link

pstrh commented Feb 10, 2022

Fixed in PR #12013

@Hew007
Copy link

Hew007 commented May 7, 2022

I also run into this issue, used CSS.escape() to resolve, waitting cra fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants