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

Composing a class from a different file does not pass that class through getLocalIdent #636

Closed
pillowfication opened this issue Nov 30, 2017 · 8 comments · Fixed by #853
Closed

Comments

@pillowfication
Copy link

What is the current behavior?
Composing a class from a different file does not pass that class through getLocalIdent.

use: [{
  loader: 'css-loader',
  options: {
    modules: true,
    importLoaders: 2,
    localIdentName: '[local]',
    getLocalIdent: (context, localIdentName, localName, options) => 'prefix-' + localName,
}, {
  loader: 'sass-loader'
}]
// foo.scss
.foo { color: red; }

// bar.scss
.bar { composes: foo from "./foo.scss"; }
import styles from './bar.scss'
console.log(styles.bar)
// -> "prefix-bar foo"

What is the expected behavior?
I expected the same behavior as I would get when I compose from the same file.

// bar.scss
.foo { color: red; }
.bar { composes: foo; }
import styles from './bar.scss'
console.log(styles.bar)
// -> "prefix-bar prefix-foo"
@alexander-akait
Copy link
Member

@pillowfication can you describe use case?

@franjohn21
Copy link

franjohn21 commented Mar 14, 2018

+1 Also seeing this issue.

This breaks getLocalIdent as it cannot reliably be used on all selectors.

The use case we have is that we are using a custom getLocalIdent function to produce a hash within the classname that is deterministic in our ecosystem, both on the server side and in the browser (using the file path, as css-loader currently does, breaks us in certain circumstances).

Edit: Same issue has been reported before: #524

@pillowfication
Copy link
Author

I've been using a custom getLocalIdent function to generate my own minified selectors, but because of this issue, it doesn't work if cross-file composition is used.

I've been using the following workaround:

/* styles.css */
.my-class {
  // Note the compositions here, even though they are applied in another file
  // composes: foo bar from "./other-file.css";
  color: red;
}
/* compose-styles.js */
import { foo, bar } from './other-file.css'
import styles from './styles.css'

// Apply the workaround
styles.myClass = `${styles.myClass} ${foo} ${bar}`

export default styles
/* Component.jsx */
import styles from './compose-styles.js' // TODO: switch to `styles.css` when possible
const myClass = styles.myClass

@franjohn21
Copy link

Thanks @pillowfication that makes sense for a work around.

@evilebottnawi - what other information would be helpful here? getLocalIdent is broken with this issue.

On the server side we use css-modules-require-hook and it's generateScopedName option which is in essence the same function as getLocalIdent. However css-modules-require-hook's implementation works correctly and the generateScopedName function is called for cross-file composed selectors.

Is it possible css-loader is missing a plugin from its implementation?

@alexander-akait
Copy link
Member

@franjohn21 PR welcome 👍

@timswalling
Copy link

@pillowfication @franjohn21 I've just come across what appears to be the same problem, but it only occurs when I'm using the extract-text-webpack-plugin.

Is this case for you too?

@franjohn21
Copy link

@timswalling What version of webpack are you using? We didn't debug exactly what fixed it but this appears to be working for us in webpack @ 3.11.0 whereas it did not in webpack @ 3.10.0.

@timswalling
Copy link

@franjohn21 thanks for your help. Unfortunately I've tried with webpack @ 3.11.0 and 3.12.0 (and css-loader @ 0.28.11) and it's still happening.

It's sounds like it's a slightly different problem to the one you've got, so I'll open a separate issue.

@alexander-akait alexander-akait added this to the 2.0.0 milestone Dec 5, 2018
@alexander-akait alexander-akait mentioned this issue Dec 5, 2018
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants