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

Let Jest handle all file types #1197

Merged
merged 3 commits into from
Dec 7, 2016
Merged

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Dec 7, 2016

In 0.8.x, we switched to treat all non-JS/CSS extensions as files by default (#1059, #1077).

However when we tried to do the same in Jest, we made a mistake which introduced false positives (#1145, #1147).

We temporarily reverted this change in #1149 and reopened #667 (comment). @cpojer suggested to use transform instead of moduleNameMapper so that we can match against the full paths.

This PR implements that. I verified that the following cases work:

Fixes #667.

'^.+\\.(?!(js|jsx|css|json)$)[^\\.]+$': resolve('config/jest/fileTransform.js'),
},
transformIgnorePatterns: [
'/node_modules/.+\.(js|jsx|json)$'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed this because otherwise we can't import CSS from node_modules (e.g. bootstrap/dist/bootstrap.css). Would it make sense to change the default to that?

@gaearon gaearon added this to the 0.8.2 milestone Dec 7, 2016

module.exports = {
process(src, filename) {
return 'module.exports = ' + JSON.stringify(path.basename(filename)) + ';';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this not include the extension name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it does!

 require('path').basename('aas.asa')
'aas.asa'

@cpojer
Copy link
Contributor

cpojer commented Dec 7, 2016

nice!

* Fix exclusion regex to also exclude files without extension
* Be over-cautious with Windows paths because I'm not sure how Jest handles them
@gaearon
Copy link
Contributor Author

gaearon commented Dec 7, 2016

Tested with:

import React, { Component } from 'react';
import appCss from './App.css';
import depCss from 'postcss-discard-overridden/test/input.css';
import appSVG from './logo.svg';
import depSVG from 'postcss-load-options/logo.svg';
import appNoExt from './noext';
import depNoExt from 'abbrev/LICENSE';
import appUnknown from './lol.unknown';  // I added this
import depUnknown from 'abab/lol.unknown'; // I added this
import depJS from 'abab/lol'; // I added this, it's just module.exports = function() { return `haha` }

class App extends Component {
  render() {
    return (
      <div className="App">
        app css: {JSON.stringify(appCss)}
        <br />{'\n'}
        dep css: {JSON.stringify(depCss)}
        <br />{'\n'}
        app svg: {appSVG}
        <br />{'\n'}
        dep svg: {depSVG}
        <br />{'\n'}
        app noExt: {appNoExt}
        <br />{'\n'}
        dep noExt: {depNoExt}
        <br />{'\n'}
        app unknown: {appUnknown}
        <br />{'\n'}
        dep unknown: {depUnknown}
        <br />{'\n'}
        dep js {depJS()}{depJS.toString()}
      </div>
    );
  }
}

export default App;

(Need to add something like this to integration test, see #1187)

npm start

screen shot 2016-12-07 at 20 13 12

npm test

screen shot 2016-12-07 at 20 13 37

npm start post ejection

screen shot 2016-12-07 at 20 15 19

npm test post ejection

screen shot 2016-12-07 at 20 15 25

@gaearon gaearon merged commit 5456fff into facebook:master Dec 7, 2016
@EnoahNetzach
Copy link
Contributor

I already added this to the todo list of #1187 ;)

@gaearon gaearon deleted the jest-transform branch December 7, 2016 21:13
@gaearon gaearon mentioned this pull request Dec 7, 2016
alexdriaguine pushed a commit to alexdriaguine/create-react-app that referenced this pull request Jan 23, 2017
* Let Jest handle all file types

* Update regexes

* Fix exclusion regex to also exclude files without extension
* Be over-cautious with Windows paths because I'm not sure how Jest handles them

* There is no automatic babel-jest discovery now that we use transsform
randycoulman pushed a commit to CodingZeal/create-react-app that referenced this pull request May 8, 2017
* Let Jest handle all file types

* Update regexes

* Fix exclusion regex to also exclude files without extension
* Be over-cautious with Windows paths because I'm not sure how Jest handles them

* There is no automatic babel-jest discovery now that we use transsform
@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants