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

2.0 Discussion #162

Closed
timarney opened this issue Dec 9, 2017 · 98 comments
Closed

2.0 Discussion #162

timarney opened this issue Dec 9, 2017 · 98 comments

Comments

@timarney
Copy link
Owner

timarney commented Dec 9, 2017

TLDR:
See Readme notes for alternatives to this repo:

Also see - #343 as of Create React App 2.1.2 (there's a breaking issue) versions prior not using the utility functions from this repo still worked.


Opening this to discuss work to be done in a 2.0 release.

One thing I would like thoughts on dropping support for older versions of Webpack.

i.e. this part of code

// Older versions of webpack have `plugins` on `loader.query` instead of `loader.options`.
  const options = loader.options || loader.query;
  options.plugins =  [pluginName].concat(options.plugins || []);
  return config;
@Guria
Copy link
Collaborator

Guria commented Dec 9, 2017

We should invest some time into tests. But it isn't blocks major release I suppose

@timarney
Copy link
Owner Author

Would love to get some tests in place. If you have any proposal as far as best setup for that would be +1

@Guria
Copy link
Collaborator

Guria commented Dec 18, 2017

We also can drop passing process.env.NODE_ENV as argument to rewires.

@timarney
Copy link
Owner Author

As per my note here
mobxjs/mobx-react#381 (comment)

should make some adjustments for plugin ordering i.e. https://github.com/loganfsmyth/babel-plugin-transform-decorators-legacy#note-order-of-plugins-matters

@andriijas
Copy link
Collaborator

Okey lets get the party started, i have a pending PR,

@timarney can you create a next branch of react-app-rewired and we can accumulate changes there?

@timarney
Copy link
Owner Author

There's a branch now.

@andriijas
Copy link
Collaborator

andriijas commented Jan 18, 2018

react-scripts 2.0 is happening, more info here:
facebook/create-react-app#3815

When CRA2 is out:

  • Update README to make it clear that react-app-rewired 2.0 only works with react-scripts 2.0
  • Update LESS readme to point to CSS module documentation in CRA

@Guria
Copy link
Collaborator

Guria commented Jan 18, 2018

Is it critical to drop [email protected] support?
Do we expect critical config changes on CRA side that hard to manage on rewired side?

@andriijas
Copy link
Collaborator

andriijas commented Jan 18, 2018

Not critical but Id argue that there is no reason to upgrade rewire if you are holding of react-scripts.

One of the new features in cra2 is css modules so naturally the less rewire should support it in rewire 2 but it will be a lot more job to add css module support to older versions because we only extend the config, not adding anything new.

react-scripts 1.x => rewire 1.x , react-scripts 2 => rewire 2.x feels like a natural mapping which is easy for users to get.

@pauliusuza
Copy link

Would it be possible for you to publish a test version of react-app-rewired under @next in npm?

@andriijas
Copy link
Collaborator

@timarney now that there are changes that breaks rewire with cra2, can we have a pre release? I just pushed a minor tweak to next branch.

@timarney
Copy link
Owner Author

timarney commented Feb 7, 2018

Should be there now:

next: 2.0.0
➜ react-app-rewired git:(next)

@andriijas
Copy link
Collaborator

Thanks.

I figured its better to hold of cleaning of supporting old webpack config formats untill webpack 4 is in cra2.

@unconfident
Copy link
Contributor

After thinking about #204 for a while I think I'd like to see better utility for rule/loader list traversal and manipulation. What I'd like to be able to out of the box with utils provided by rewire v2 or earlier is:

  • Improved search logic that resembles Webpack's logic more closely.
    Basically Expand getLoader method to iterate over rule.loaders and rule.loader when applicable #204, will try to submit PR sooner or later

  • Being able to find every rule that includes specific loader instead of just one

  • Proper destinction between a Rule and UseEntry, searching Rule by loader name or use entry

  • Inserting loader before or after specific rule

    • With an automatic conversion of a shorthand to list style. I.e. converting

      {
        test: /\.jsx$/,
        loader: 'babel-loader',
        options: { ... } 
      }

      to

      {
        test: /\.jsx$/,
        use: [
          { loader: 'babel-loader', options: { ... } }
        ]
      }
  • Changing loader options that would transparently convert string shorthand to a UseEntry object. I.e.

    { test: /\.jsx$/,
      use: [
        'babel-loader'
      ] }

    to the code block above

@timarney
Copy link
Owner Author

Noting this for discussion

What I'd like to propose is adding another function to the main package that would simillarly to injectBabelPlugin locate rules using babel-loader and switch babelrc option to true.
This way it will be both, explicit and backwards compatible

#203

@nfantone
Copy link

nfantone commented Apr 24, 2018

@timarney Hi! Apologies for hi-jacking the discussion, but today I tried using 2.0.0 (+ latest [email protected]) and while the build command worked with no noticeable issues, test fails complaining about babel 6.26.0 usage.

Requires Babel "^7.0.0-0", but was loaded with "6.26.0". If you are sure you have a compatible version of @babel/core, it is likely that something in your b
uild process is loading the wrong version. Inspect the stack trace of this error to look for the first entry that doesn't mention "@babel/core" or "babel-core"
to see what is calling Babel.

Using regular react-scripts test works as expected. Any advice on this? I could create a new issue, if needed.

EDIT: I can confirm that adding the Babel 7 bridge fixes the problem for me.

❯ yarn add @babel/core@^7.0.0-0 --dev
❯ yarn add [email protected] --dev

Feels like this should not be necessary, though.

@timarney
Copy link
Owner Author

timarney commented Apr 24, 2018

@nfantone ... @andriijas might be able to shed some light but there really hasn't been any work done on the next version for a while and it's on the back burner for me for now.

@andriijas
Copy link
Collaborator

@nfantone you should be able to use react-app-rewire 1.x just fine with the cra 2.0 alpha. i also have a lot of other things going on right now and i think its easier to move forward with rewire 2 once cra 2 is more finalized.

@terminalqo
Copy link

Need conversations search input box。

@unconfident
Copy link
Contributor

By the way, I made that thing I suggested in my previous comment into a separate package of its own: @unconfident/webpack-module-rules-utils

I think it can be useful outside of react-app-rewired too. For instance Next.js' webpack configuration is also meant to be customized in a similar way at runtime. Another one I can recall would be Storybook's "full control mode" and there are likely some other projects out there that suggest manipulating Webpack configuration at runtime

It has not yet been covered with tests and also lacking documentation at the moment. So if you're going to use it, use it with caution, and either rely on intellisense of your code editor or take a look at the source code for the list of method names and their signatures.

sesam added a commit to sesam/react-app-rewired that referenced this issue Jul 25, 2018
Some react version compatibility is missing. Not sure how that should be noted. Maybe also add some link to timarney#162 so people coming in with react or react-native apps that can't be rewired won't send in unwanted bug reports about things not working that are not supposed to be working.
sesam added a commit to sesam/react-app-rewired that referenced this issue Jul 25, 2018
Some react version compatibility is missing. Not sure how that should be noted. Maybe also add some link to timarney#162 so people coming in with react or react-native apps that can't be rewired won't send in unwanted bug reports about things not working that are not supposed to be working.
@sesam sesam mentioned this issue Jul 25, 2018
@timarney
Copy link
Owner Author

timarney commented Sep 1, 2018

Note: If anyone has interest in leading / maintaining a 2.0 release @ me .

My time has been and will be limited for this project. That is to say considering not doing 2.0 given I can't support it . Thanks to all for helping out to date.

@andriijas
Copy link
Collaborator

Thx for the update. Im currently not involved in any react project therefor not putting any time on the tools I worked on last spring.

@AndyOGo
Copy link

AndyOGo commented Sep 3, 2018

what a bummer, this project is a great idea

@ndbroadbent
Copy link

It's worth noting that these configurations still works with CRA2.1.1 and react-app-rewired, we continue to use it without no problem in our application.

OHHHH. Ahaha well I guess that was a waste of time. The README says "This repo doesn't support CRA 2.0", so I kind of assumed that it was broken.

GitHub also didn't help by collapsing ~30 comments in the middle, so I didn't see anything about customize-cra. For some reason I thought GitHub only collapsed the "+1" and "me too" comments.

I might just use your config then. Thanks!

@ndbroadbent
Copy link

Sorry I just wanted to follow-up one last time and say that I actually really like craco to configure create-react-app. The architecture is awesome, and it's very easy to install and get it running. I also decided to release a few plugins as NPM packages:

  • craco-antd - For Ant Design, including Less and babel-plugin-import
  • craco-less - Adds less-loader to the webpack config

I think my craco.config.js file is super clean and just does exactly what I need:

const CracoAntDesignPlugin = require('craco-antd');
const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer');
const WebpackBar = require('webpackbar');

module.exports = {
  webpack: {
    alias: { react: 'preact-compat', 'react-dom': 'preact-compat' },
    plugins: [
      new BundleAnalyzerPlugin(),
      new WebpackBar({ profile: true }),
    ],
  },
  plugins: [{ plugin: CracoAntDesignPlugin }],
};

Thanks @abenhamdine for the tip about BundleAnalyzerPlugin and WebpackBar, those are awesome!

screen shot 2018-11-13 at 5 33 33 am

@abenhamdine
Copy link

Sorry I just wanted to follow-up one last time and say that I actually really like craco to configure create-react-app. The architecture is awesome, and it's very easy to install and get it running. I also decided to release a few plugins as NPM packages:

That's great ! I like craco design too, but I failed to make less loader work last time I tried craco, thus I'm eager to try your plugins.
Thx again ! 💃

@abenhamdine
Copy link

Sorry for the noise, I just want to let know that craco and craco-antd plugin work perfectly with cra 2.1.1/antd/typescript so craco looks like a good alternative.

In another hand, react-app-rewired is also still working fine for us with cra 2.1.1/antd/typescript (maybe some react-app-rewired-xxx plugins are breaking with cra 2.X but it didn't affect us).

@marcopeg
Copy link
Contributor

Hi guys, I have tried a different approach and followed the guidelines proposed in CRA to extend it by forking the react-script package and providing an interface that is similar to rewired and craco, but that doesn't require any changes in the scripts:

https://github.com/marcopeg/create-react-app/blob/master/packages/react-scripts/README.md

I'd love to hear some feedbacks and maybe get collaboration in porting few plugins into it.

@kopax
Copy link

kopax commented Nov 19, 2018

Hi, I have started a react app with CRA v2.0, this is how I have edited my scripts in package.json:

  "scripts": {
    "start": "react-app-rewired start",
    "build": "react-app-rewired build",
    "test": "react-app-rewired test --env=jsdom",
    "eject": "react-scripts eject",
  },

Now that I run npm test, I have the following error:

> [email protected] test /home/dka/workspace/www
> react-app-rewired test --env=jsdom

internal/fs/watchers.js:173
    throw error;
    ^

Error: ENOSPC: System limit for number of file watchers reached, watch '/home/dka/workspace/www/node_modules/babel-template'

Is this really not working with CRA v2. ? Why ?

Should we use react-app-rewired test --env=jsdom for running the test ? Why react-scripts test is not working?

@AndyOGo
Copy link

AndyOGo commented Nov 19, 2018

@kopax
Just try https://github.com/sharegate/craco as first mentioned above #162 (comment)

@kopax
Copy link

kopax commented Nov 19, 2018

Well, my issue was a wrong parameter of fs.inotify. Solved. And so far I am using react-app-rewired only for plugin babel-plugin-styled-components, it seems to work fine with CRA v2. I've also seen a few people saying it work fine here, so I presume craco is not required.

@dawnmist
Copy link
Collaborator

dawnmist commented Nov 20, 2018

@kopax The core functions work with CRA2.x, but many (most?) of the pre-existing rewires do not, because they were written for Webpack 2/3, and the Webpack 4 configuration has altered significantly - many of those rewires break. The decision finally was that there should be a split to a new project for CRA2.x rather than continuing react-app-rewired so that old rewires that were no longer maintained and were not CRA2.x compatible were not mistakenly thought to be working.

Two potential splits have emerged - one being craco and the other being customize-cra. Customize-cra uses the core functionality from react-app-rewired, but provides new rewires written for CRA2.x. Craco provides that functionality in a separate way without requiring react-app-rewired.

@comerc
Copy link
Contributor

comerc commented Nov 23, 2018

@Timer just add API-hook, like GatsbyJS onCreateWebpackConfig. Please! I like CRA, but GatsbyJS leading.

@jbergens
Copy link

The things I've used rewired for is mostly getting decorators to work. It would also be nice to get to override some other things like eslint config.

@jamesmfriedman
Copy link

For anyone just manually overriding the config, this works 100% fine with CRA2, using it in production with many projects.

@Gamezpedia
Copy link

i just updated to "react-scripts": "^2.1.1", and it stopped working.

@jihchi
Copy link

jihchi commented Dec 25, 2018

@Gamezpedia #343

@AndyOGo
Copy link

AndyOGo commented Dec 26, 2018

@jihchi
@jamesmfriedman
@Gamezpedia
Just switch to CRACO JS (crafted for CRA V2)
https://github.com/sharegate/craco

@abenhamdine
Copy link

abenhamdine commented Dec 26, 2018

Just switch to CRACO JS (crafted for CRA V2)

One should know that https://github.com/sharegate/craco is now broken with cra 2.1.2, as this repo is, see dilanx/craco#61

@AndyOGo
Copy link

AndyOGo commented Dec 26, 2018

That's a bummer. At least it was written for CRA V2, so it should at least be more likely that it is maintained and will be fixed sooner (fingers crossed)...

@jamesmfriedman
Copy link

Again, no need to switch. This repo continues to work perfectly for CRA 2.1.1

@abenhamdine
Copy link

abenhamdine commented Dec 27, 2018

Again, no need to switch. This repo continues to work perfectly for CRA 2.1.1

You're right, sorry, I read CRA 2.1.2 instead of 2.1.1, I edited my previous comment accordingly #162 (comment)

This repo still works perfectly for us too until <= cra 2.1.1

@edpark11
Copy link

edpark11 commented Dec 29, 2018

Just a note-- as an alternative with CRA 2.0, I tried https://github.com/jpavon/react-scripts-ts and found that it works perfectly with this version of react-app-rewired and allows for an upgrade to Webpack 4. It also seems to compile projects much faster than CRA 2.0. I have no association with jpavon's project other than I tried it and liked it, YMMV.

@timarney
Copy link
Owner Author

timarney commented Jan 6, 2019

I've published a 2.0 version to fix issues with CRA v2.1.2 .

Thanks @tiffon + others who helped out. Utility helper rewires have been removed.

If you must rewire this should get things back to a working state.

Not other updates are planned this was just a break fix situation.

Also with that I'm going to close this issue.

@timarney timarney closed this as completed Jan 6, 2019
@AndyOGo
Copy link

AndyOGo commented Jan 6, 2019

seems rescripts is working:
https://github.com/rescripts/rescripts

@jamesmfriedman
Copy link

Thanks @timarney! Myself and other others I work with are very reliant on this project so I’d be happy to contribute to keep it going.

I rely solely on custom requiring in config overrides, I’ve become a bit of a surgeon in regards to tweaking the CRA build process :).

Love this project, thanks for your hard work.

@smmoosavi
Copy link

You can use monkey-react-scripts

It lets you mutate webpack config and change babel config for tests

module.exports = function(webpackConfig, isDevelopment) {
  // mutate webpackConfig
};

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