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

Lightweight eject #298

Closed
davincho opened this issue Jun 19, 2017 · 8 comments
Closed

Lightweight eject #298

davincho opened this issue Jun 19, 2017 · 8 comments

Comments

@davincho
Copy link

I really like this project and the philosophy of not limiting the user by anything.
Escape hatches via .babelrc and razzle.config.js are good, but I somehow miss full control over the config. I know that I could easily fork this project and adapt the code to my needs, but this is not my goal (probably I am totally wrong and forking is this way to go).

To explain my problem, take a look at the following use case. Let's say I want to handle SVG files differently in webpack than other media files (for instance with svg-inline-loader).

As svg has been defined in

test: /\.(jpg|jpeg|png|gif|eot|svg|ttf|woff|woff2)$/,
(Note: This is an older version of create-config.js but it is still good to explain the problem), I had to do the following in razzle.config.js:

module.exports = {
  modify: (config, { target, dev }) => {
    // ...
    const loaders = [
      {
        test: /\.svg$/,
        use: [
          {
            loader: 'svg-inline-loader'
          }
        ]
      }
    ];

    // Exclude svg from default config
    // Dangerous as we are depending on the order of the predefined rules (!)
    config.module.rules[2].test = /\.(jpg|jpeg|png|gif|eot|ttf|woff|woff2)$/;

    config.module.rules = loaders.concat(config.module.rules);
    // ...
    return config;
  }
};

As already mentioned in the comment, this adaption is error prone, as it will break if any new rules are added or the order of them are changed. It would be cool to have something like an eject command in create-react-app, but probably only exposing the resulting webpack.config.js for prod and dev. Any thoughts?

@jaredpalmer
Copy link
Owner

You are correct that this particular way of modifying razzle is very error prone and fragile. Instead of using the Array[i]. I suggest using Array.find or Array.findIndex. This is much more future proof as it will continue to work, even if a rule changes position.

As for razzle eject, it's totally possible to build. To make life easier we would probably want to refactor create-config into 4 webpack.config...js files. Similarly to CRA, I'd want to setup a form asking why people are ejecting in the first place.

@jariz
Copy link
Collaborator

jariz commented Sep 6, 2017

Quoting @jaredpalmer: (@ #325)

So if we end up refactoring this mess (and i agree its a mess). I think we should go with 5 files:

  • webpack.client.dev.js
  • webpack.client.prod.js
  • webpack.server.dev.js
  • webpack.server.prod.js
  • webpackDevServer.config.js

@dtothefp
Copy link

dtothefp commented Sep 7, 2017

I don't like eject...it defeats the purpose of extracting configuration into a module. I like that you are providing hooks with modify. One option is instead of providing this hook as a POJO that you export that hook could extend a class from the Razzle module. If this class is an EventEmitter, you can emit the config and the child class can have an on method and mutate the Webpack config in memory. This allows for changing configuration in the same way without ejecting https://github.com/build-boiler/making-gulp-suck-less/blob/master/packages/gulpy-boiler-task-eslint/src/eslint.js#L77

@jariz
Copy link
Collaborator

jariz commented Sep 7, 2017

Maybe we can do both? A modify option and an eject on top?
I do prefer the modify option, but a question I've had before from colleagues while using razzle, is simply that they want to be able to eject if their project needs to scale that much that a modify option wouldn't make much more sense (and actually be more complicated than a eject)

For projects like CRA it makes sense to just only have a eject (I guess), but razzle has 5 different types of configs, so things will get overwhelming quick if that's the only option we provide.

About the whole EventEmitter thing, how is this that much different from the current modify function we have? What advantages does it add?

@dtothefp
Copy link

@jariz I guess it's not that much different than the modify function I have just used it in the past to have a directory or /tasks and then just require all the tasks in that directory. All of these tasks extend some sort of class that extends the EventEmitter. Therefore the internal task, in this case Webpack can emit it's configuration just before creating the compiler and the task in the project can hook into this event. Doesn't really matter here because Razzle essentially just has one task which is webpack, but it's just something I've played around with.

A simple example might be something like this:

import {RazzleTask} from 'razzle';

class MyProject extends RazzleTask {
   constructor() {
      super();
      this.on('webpack:modify', (config) => {
         config.module.use.push(/* some loader */);
      })
   }
}

This would mutate the config and allow your task that did the emitting to continue on with the new config. Mutation is not amazing but ejecting is worse in my opinion. It's hard to make these extracted build modules configurable.

I have an example here if you're interested

https://github.com/build-boiler/making-gulp-suck-less/blob/master/packages/gulpy-boiler-utils/src/TaskHandler.js

@romainquellec
Copy link
Contributor

Any news on this ?

@jariz
Copy link
Collaborator

jariz commented Jan 3, 2018

I much prefer keeping the modify function and adding a plugin system on top like suggested in #437.

I'm not fan of adding a eject for the following reasons:

  • We'd need to do a rather huge internal refactor (as mentioned above)
  • Ejecting has all sorts of disadvantages such as it generating an overwhelming amount of webpack configs and the fact that once you eject, you cannot go back. You'll also loose any updates that razzle brings.
  • The plugin system will solve the problem of modify functions getting too complex, anyway.

@jchook
Copy link

jchook commented Jan 6, 2019

You lose a lot of buy-in when you refuse to provide an easy opt-out.

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

6 participants