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

Plugins added by rewiring do not appear to work in test #120

Closed
gertsonderby opened this issue Oct 26, 2017 · 11 comments
Closed

Plugins added by rewiring do not appear to work in test #120

gertsonderby opened this issue Oct 26, 2017 · 11 comments

Comments

@gertsonderby
Copy link

The rewiring npm package react-app-rewire-styled-components adds the styled-component plugin to Babel. This works excellently in the browser, but does not appear to have any effect in test. This seems to come back to react-app-rewired according to @mxstbr.

I have created a repo with a test that shows this. In this repo, running the npm test command has a failing test. Add the following .babelrc file in the project root to make it pass:

{
  "plugins": ["styled-components"]
}

This pretty strongly indicates some misconfiguration occurring in the Jest setup, I think?

@timarney
Copy link
Owner

Looks like there is an issue but first I'm under the impression that adding that plugin affects the className output to make it more readable would a test for a className be more suitable?

cc: @Guria might need some input from you to help track this one down.

@gertsonderby
Copy link
Author

gertsonderby commented Oct 27, 2017

It also sets up display name. At any rate, testing className would show the same result - the plugin doesn't get at the className either. And as mentioned, adding a .babelrc makes the test pass.

@timarney
Copy link
Owner

timarney commented Nov 4, 2017

@Guria can you take a look at this one?

Thinking it would be a good time to release a major version and remove the babelTransform file + document the jest config.

I tried locally using this repo https://github.com/timarney/mobx-rewire-test and I can't get it to pick up the babel-plugin-transform-decorators-legacy plugin for the jest test.

Update - mobx-rewire-test is working again separate issue.

@timarney
Copy link
Owner

@dawnmist any chance you might have some insight on this?

@dawnmist
Copy link
Collaborator

dawnmist commented Nov 25, 2017

Looks like the test script isn't actually calling the function to modify the config at all (test.js lines 11-13).

Once it is modified so that the config function is being called there is a second issue where the babel loader injection in react-app-rewire-styled-components is failing due to config.modules not existing in the modified config. It's set up to expect a set of webpack config options, and those simply don't exist in the config that react-scripts is providing for test. Instead of the config.module, they're directly setting the babel loader transforms in config.transform, so any call to add the styled-components plugin to the babel-loader fails because there is no babel-loader being defined in the config.

@dawnmist
Copy link
Collaborator

dawnmist commented Nov 25, 2017

It looks like it is a deliberate exclusion for running the normal config modification function in test mode - because in test mode it is not creating a Webpack config, it's creating a Jest config and the two are not compatible. So that's not a bug - to modify the Jest config the config-overrides.js file should use the object form and set the jest function:

module.exports = {
  webpack: (config, env) => {
    config = rewireStyledComponents(config, env);
    return config;
  },
  jest: (config) => {
    ...your modifications...
    return config;
  }
}

The jest config by default is setting things up to use the .babelrc file to configure additional babel plugins. So the workaround in the initial comment of setting the plugins inside .babelrc is actually the correct way to add babel plugins for the Jest test mode. By using the .babelrc to define the plugins to use when testing with Jest, if that was the only transform required you can avoid needing to even define the jest config option above.

So overall - I think this is actually working as designed at present, it's just confusing initially because the way to configure Jest is different to the way to configure Webpack, and the rewires used for Webpack do not and cannot work directly for the Jest configuration. Instead, the Jest configuration gets altered by react-app-rewired to permit defining babel plugins in .babelrc.

@timarney
Copy link
Owner

timarney commented Nov 25, 2017

Thanks for taking the time to dig into that - the .babelrc works well at a user level (need to document it).

I was trying to see if I could get something working at the rewire level.


@Guria @mxstbr @osdevisnot @icopp would like to get your thoughts on this:

Maybe we just get rewire owners to document that the plugin will need to be added to the .babelrc in order for Jest to use the plugin for tests. This repo will only be responsible for flipping the .babelrc to true.

jest: (config) => {
    ...your modifications...
    return config;
  }

Some Background:

Initially wrote the MobX rewire I ended up pushing the transform-decorators-legacy manually in the babelTransform.js file. So the end user could just install the rewire and mod the config. I knew at the time that was a hack as you can't modify that without changing that file. Not a proper long term solution.

Ideally if there was a nice way to allow rewire owners to push plugins here that would be nice.

require.resolve('babel-plugin-transform-decorators-legacy');
customPlugins.push('babel-plugin-transform-decorators-legacy');
module.exports = babelJest.createTransformer({
  presets: [require.resolve('babel-preset-react-app')],
  plugins: customPlugins,
  babelrc: true
});

@dawnmist
Copy link
Collaborator

The config.plugins doesn't work - jest does not pass the config object to babelTransform, so there isn't a way that I can see to get the value of the plugins field into the created babel transform. Jest also insists that custom transforms must be referenced as a file, so you cannot create it inline as part of the initial config.

I've tried doing it as an environment variable - but that doesn't get passed to the running babelTransform.js process when it gets called by Jest, so it didn't work (I thought it had initially, but that was because I had a babel section left in the package.json from testing yesterday).

The only other option I can see would be to generate a temporary file during the configuration stage, that implemented the babelTransform.js with the list of plugins to use based on the project's overrideConfig jest function. The list of plugins could then be generated by using an environment variable that gets hard-coded into an appropriate array in the temporary babelTransform.js file (i.e. use the existing one as a template and fill in the customPlugins array when writing the temp file). That's also pretty hacky. :(

@timarney
Copy link
Owner

Thanks for digging further. I was doing some testing as well and was getting the validation errors for the Jest config (plugins).

Best to just add a note about the .babelrc for testing with Jest in my opinion vs another hack.

@timarney
Copy link
Owner

Added a new issue to document Jest options #146

@timarney
Copy link
Owner

Going to close this. Can pick the conversation up in #146 as need be.

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

3 participants