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

Sass support in jest test files appear to be broken #4345

Closed
Tasemu opened this issue Apr 23, 2018 · 13 comments
Closed

Sass support in jest test files appear to be broken #4345

Tasemu opened this issue Apr 23, 2018 · 13 comments

Comments

@Tasemu
Copy link

Tasemu commented Apr 23, 2018

Is this a bug report?

Yes

Did you try recovering your dependencies?

Yes

(Write your answer here.)

Which terms did you search for in User Guide?

Sass support

(Write your answer here if relevant.)

Environment

  1. node -v:
  2. npm -v:
  3. yarn --version (if you use Yarn):
  4. npm ls react-scripts (if you haven’t ejected):

Then, specify:

  1. Operating system:
  2. Browser and version (if relevant):

Steps to Reproduce

(Write your steps here:)

1: Update react-scripts to latest release
2: Import styles from 'component.module.scss' in component file
3: Import styles from 'component.module.scss' in component.test.js

Expected Behavior

(Write what you thought would happen.)

Classnames are imported for component file and test file correctly for comparison

see #3815

Actual Behavior

(Write what happened. Please add screenshots!)

Component imports classNames correctly, test file classnames are undefined

Reproducible Demo

(Paste the link to an example project and exact instructions to reproduce the issue.)

@gaearon gaearon added this to the 2.0.0 milestone Apr 23, 2018
@Aftabnack
Copy link
Contributor

This is because we don't have scss preprocessor configured for jest. And I am guessing the same problem will be there when we use .module.css files.

Can I work on this?

@Tasemu
Copy link
Author

Tasemu commented Apr 23, 2018

I'm going to attempt a PR too :)

@Tasemu
Copy link
Author

Tasemu commented Apr 24, 2018

Would it be a good idea to follow the 'mocking css modules' section of the jest-webpack documentation found here?

https://facebook.github.io/jest/docs/en/webpack.html#mocking-css-modules

This would allow tests to check that correct classNames were being applied without having to set up preprocessor transforms.

@Timer
Copy link
Contributor

Timer commented Apr 24, 2018

This is odd, because we already configured that for CSS Modules (and I believe we even have tests); I wonder why Sass isn't working (or is just not what's expected)?

@Timer
Copy link
Contributor

Timer commented Apr 24, 2018

Ah, it appears we need to add {sass,scss} to these locations:

transformIgnorePatterns: [
'[/\\\\]node_modules[/\\\\].+\\.(js|jsx|mjs)$',
'^.+\\.module\\.css$',
],

'^.+\\.module\\.css$': 'identity-obj-proxy',

@Tasemu
Copy link
Author

Tasemu commented Apr 24, 2018

I'd be happy to make a PR on this, one question however (I cannot seem to find an answer in the documentation I have searched), How am I supposed to test my change with the cloned repo? I cannot see any way to test the changes with a generated create-react-app. Also if I were able to generate an app using the changes, shall I just have the extra generated files not committed then make a PR? Sorry in advance if these are dumb questions. Fairly new to the contributing thing.

@iansu
Copy link
Contributor

iansu commented Apr 24, 2018

You can test your changes by running yarn create-react-app my-app from the base of your cloned repo. That will create a new app called my-app using your local changes.

You should not commit my-app when submitting a PR. You can create my-app outside of your cloned repo by doing something like this instead: yarn create-react-app ../my-app.

@Tasemu
Copy link
Author

Tasemu commented Apr 24, 2018

It appears that after modifying createJestConfig.js and generating an app, doing the following:
import styles from test.module.scss
results in:
console.log(styles) // => 'test.module.scss'

So it appears to be importing the sass module as a string of the file name. I'm going to continue looking into this, if this pops out to you as something obvious then feel free to let me know. :)

@Tasemu
Copy link
Author

Tasemu commented Apr 24, 2018

I think I have an idea, going to test it tomorrow as generating the node_modules folder every time for CRA is murdering my mobile internet.

@Tasemu
Copy link
Author

Tasemu commented Apr 25, 2018

it also appears that importing css modules is not working as expected either.

import styles from './test.module.css';
import sass from './sass.module.scss';

it('should handle css modules', () => {
  console.log(styles) // => {}
})

it('should handle sass modules', () => {
  console.log(styles) // => 'sass.module.scss'
})

Using this configuration:

transformIgnorePatterns: [
      '[/\\\\]node_modules[/\\\\].+\\.(js|jsx|mjs)$',
      '^.+\\.module\\.{css,sass,scss}$',
    ],
    moduleNameMapper: {
      '^react-native$': 'react-native-web',
      '^.+\\.module\\.{css,sass,scss}$': 'identity-obj-proxy',
    },

@Tasemu Tasemu closed this as completed Apr 25, 2018
@Tasemu Tasemu reopened this Apr 25, 2018
@Tasemu
Copy link
Author

Tasemu commented Apr 25, 2018

Accidental close and comment*

@Tasemu
Copy link
Author

Tasemu commented Apr 27, 2018

I'm unsure how to continue this unfortunately, if anyone has some ideas as to the strange behaviour above, otherwise i'll keep this here to see if anyone else wants to pick this up. :)

@okhy
Copy link

okhy commented Oct 23, 2018

it also appears that importing css modules is not working as expected either.

import styles from './test.module.css';
import sass from './sass.module.scss';

it('should handle css modules', () => {
  console.log(styles) // => {}
})

it('should handle sass modules', () => {
  console.log(styles) // => 'sass.module.scss'
})

Hello, this is still an issue which basically prevents me from testing conditional class assginment as in:

const Tile = ({ tilePlacement }) => (
  <div className={styles[`tile_${tilePlacement}`] || styles.default} />
)

will, in Jest, never return default but tile_[tilePlacementValue]

@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants