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

Plugin system #2784

Closed
wants to merge 2 commits into from
Closed

Plugin system #2784

wants to merge 2 commits into from

Conversation

Timer
Copy link
Contributor

@Timer Timer commented Jul 13, 2017

A plugin system is something which proposes to solve many problems.

For instance, we'd like to be able to add support for things like Relay, without increasing the initial installation size of the build tooling.

This could also be applied to things like TypeScript, Sass, et al.

Design goals:

  1. Zero-configuration, opt-in additional features
  2. Simple setup (yarn add react-scripts-plugin-relay)
  3. Transparent to users who eject
  4. Internal use only (not pluggable, sorry!)

We need a lot of discussion about this before we can progress further!

@Timer Timer mentioned this pull request Jul 13, 2017
2 tasks
@Timer Timer force-pushed the plugins branch 2 times, most recently from e46cdc2 to b4bca03 Compare July 13, 2017 18:46
@@ -114,13 +115,28 @@ inquirer
fs.mkdirSync(path.join(appPath, folder));
});

let addtlDeps = new Map();
Copy link
Contributor

@viankakrisna viankakrisna Jul 13, 2017

Choose a reason for hiding this comment

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

maybe rename this to additionalDeps? it's not obvious at the first time I read it....

add tl deps, what is tl? 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No harm in longer names; I'll change it. 😄

If I could have ' it'd be nice: add'l., addt'l.

addl seemed too (??), so I opted for addtl.

Copy link
Contributor

Choose a reason for hiding this comment

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

or use emojis for language-agnostic interpretation 😄

➕📦 = new Map()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Omg I could only imagine someone's face when they see a package emoji as an identifier.

@Timer
Copy link
Contributor Author

Timer commented Jul 13, 2017

Cleaned up some code and made the eject step easier to follow.

Ready for some more review.

@sompylasar
Copy link

  1. Internal use only (not pluggable, sorry!)

@Timer Hi, could you please elaborate what this means for the end users (app developers), and for the community who forks create-react-app to add one or another feature into react-scripts (hopefully future plugin developers instead of forking react-scripts)? Thanks!

@Timer
Copy link
Contributor Author

Timer commented Jul 14, 2017

@sompylasar Great question!

So, end-users (app developers) will only be able to use plugins which we approve and whitelist.
Typically, this means it meets a set of criteria:

  1. Use-case or functionality is popular
  2. Adds value
  3. Easy to maintain and underlying tools are stable
  4. We have control to modify & publish updates to the package

People who fork create-react-app will hopefully be a dying breed (not because forking is bad, but because it's no longer necessary; forking is still the best alternative to ejecting).

That said, people who create forks will be able to create and whitelist their own plugins to be consumed, but these plugins will not work with react-scripts.
This should make following upstream (this repository) much easier as you never have to change anything in your fork except for creating a plugin.

Unfortunately, it needs to be this way to ensure we can provide a cohesive, updatable experience.
Allowing anyone to create a plugin would cause way too much churn between updates to react-scripts.

@sompylasar
Copy link

@Timer Thanks!

So, end-users (app developers) will only be able to use plugins which we approve and whitelist.

Which do you plan to whitelist? TypeScript, obviously. What about CSS modules? What about SCSS? What about having SCSS and CSS modules combined?

Use-case or functionality is popular

How do you determine popularity?

What should I do as an advanced app developer who likes CRA developer experience but needs a non-whitelisted feature? Eject is not an option as I'd like to not mess with webpack configs.

Have you considered extracting the CRA developer experience (the terminal realtime UI with linter and tests) into a module pluggable to a webpack config?

That said, people who create forks will be able to create and whitelist their own plugins to be consumed, but these plugins will not work with react-scripts.

Could you please rephrase this? "Not work with react-scripts"?

Unfortunately, it needs to be this way to ensure we can provide a cohesive, updatable experience.
Allowing anyone to create a plugin would cause way too much churn between updates to react-scripts.

Understood. But I think there will be a multitude of forks with extra plugins enabled that you haven't whitelisted, as is now with the react-scripts forks. Why not give the responsibility for whitelisting to the app developers? Some of them are qualified enough to decide that the plugin works for their apps. Just keep a list of "recommended" or "compatible" plugins. The popular plugins that work will rise without your censorship, those that don't will die naturally.

I hope you understand how CRA was a bless after rolling own webpack configs. Even for an advanced dev, it's not scalable enough and it takes a couple months to roll out a system comparable to CRA by developer experience. Thank you.

@Timer
Copy link
Contributor Author

Timer commented Jul 14, 2017

Which do you plan to whitelist? TypeScript, obviously. What about CSS modules? What about SCSS? What about having SCSS and CSS modules combined?

We do not have any promised plans yet. CSS modules are most likely going to be enabled as an always-on, supported feature (no plugin needed).

Plugins can be combined, so you could add a Scss and CSS modules plugin (if we used plugins for both).

How do you determine popularity?

"Popularity" probably isn't the best metric, because we will not support everything popular.
We will basically see what is most often requested to be supported by CRA which isn't possible without access to the webpack configuration (e.g. Sass is possible without access to webpack, but we might make an exception for it). This will be determined on a case-by-case basis.

What should I do as an advanced app developer who likes CRA developer experience but needs a non-whitelisted feature? Eject is not an option as I'd like to not mess with webpack configs.

You will be in the same boat which you are now; unable to use the feature without ejecting.
I'm sorry if this sounds mean, I do not want it to come off this way -- we simply cannot support every possible feature.
If your feature makes sense, you can ask us to create a plugin to enable it.

Have you considered extracting the CRA developer experience (the terminal realtime UI with linter and tests) into a module pluggable to a webpack config?

What you are describing is called neutrino. They do this a lot better than us.

Could you please rephrase this? "Not work with react-scripts"?

I was simply reiterating that plugins will be whitelisted.
This means custom plugins will only work in forks.

Why not give the responsibility for whitelisting to the app developers? Some of them are qualified enough to decide that the plugin works for their apps. Just keep a list of "recommended" or "compatible" plugins. The popular plugins that work will rise without your censorship, those that don't will die naturally.

I believe this may very well happen in the future when the tools reach a very stable point, but right now there's simply too much churn and I cannot see this ending well.
Plugins touch way too large of a variety of things: webpack, babel, testing (Jest).

We're just not there yet.

@Timer
Copy link
Contributor Author

Timer commented Jul 14, 2017

I hope you understand how CRA was a bless after rolling own webpack configs. Even for an advanced dev, it's not scalable enough and it takes a couple months to roll out a system comparable to CRA by developer experience.

I would like to hear your thoughts and opinions on what is currently not scalable enough.
I use and know many who use CRA successfully for enterprise (massive) applications without the need to eject.
Please do not take this as an attack, but as a genuine interest in what we could do better or enable via plugins.

We're currently in the works to support TypeScript & CSS Modules, which are two largely requested features.

@cr101
Copy link
Contributor

cr101 commented Jul 15, 2017

@Timer Would a plugin similar to the gatsby-source-wordpress be approved?

@Timer
Copy link
Contributor Author

Timer commented Jul 15, 2017

I'm not sure how gatsby-source-wordpress applies to this scenario or requires webpack changes.

@cr101
Copy link
Contributor

cr101 commented Jul 15, 2017

The reason I'm asking is because gatsby-source-wordpress is a Gatsby official plugin and I was wondering if CRA would approve such plugins

@Timer
Copy link
Contributor Author

Timer commented Jul 15, 2017

It's too early for me to say what plugins may be considered; we're not even sure we want to merge a system like this yet.

This all is completely hypothetical. 😄

@sompylasar
Copy link

sompylasar commented Jul 15, 2017

@Timer Thanks for the great responses! I didn't know about neutrino, will have a look. How CRA is different from it then, besides it's by Facebook, not Mozilla?

I hope you understand how CRA was a bless after rolling own webpack configs. Even for an advanced dev, it's not scalable enough and it takes a couple months to roll out a system comparable to CRA by developer experience.

I would like to hear your thoughts and opinions on what is currently not scalable enough.

The following experience of making a build system and improving development experience for a large web app started 1.5 years ago from scratch by taking and then over time heavily upgrading some react-redux-boilerplate. We're using lots of cutting edge features of JavaScript, the front-end, and the Node ecosystem. It's likely not applicable to CRA just because of the scale, but some ideas might be useful.

Here are the features that took most of the time (in order of appearance):

  • single webpack configuration factory function for dev / prod and for the two processes of the app: api server and frontend server (I personally don't like the non-flexible "webpack.config.{dev|prod}.js" file approach, our build system lives in a dedicated directory; and I somehow knew I will be doing universal webpack later)
  • build progress and status output
  • module require from a common module roots without relative paths, e.g. import foo from 'acme-app-foo'; from src/modules/acme-app-foo/ (I made a few roots for different subsystems of the app, so it's more like src/common/, src/api/, src/frontend/ that made things more complicated)
  • eslint (lots of plugins and tweaks over time)
  • SASS with CSS modules
  • sass-lint
  • SVG import that returns either URL or HTML of the image (I made it based on filename: .svg returns HTML, .url.svg returns URL; can be vice versa)
  • server-side rendering with styles of the rendered components inlined in a style tag (isomorphic-style-loader)
  • server-side rendering loop that waits for every async redux-saga to resolve (it's indirectly related to build system: redux-saga is based on generators and we also use async-await a lot)
  • universal webpack with hot reloading of both server-side and browser-side, and a shared terminal output for them (I made this possible with two concurrent webpack processes, but it was huge; I haven't reached the level of CRA for terminal updates)
  • universal code splitting
  • TypeScript + eslint (required a custom "proxy" parser to route to typescript parser or babel parser); I don't like tslint for not being eslint-compatible and having a smaller number of rules.

Useful things we added to the developer experience:

  • styleguide page auto-collected from all the UI components (each defines its own set of test cases)
  • TODO comments are collected from the app source code into a markdown file in a precommit hook via husky;
  • mocha test cases can be filtered via a command line arg by their full title combined from describe/it.
  • a few code metrics can be collected by running separate tooling scripts: line count, styleguide coverage, code complexity, redux-connected components.

Note that we haven't had time to implement the nice realtime terminal as we've already spent a lot on internal infrastructure making the above. The tests were also made to run via plain mocha and the default reporter, without the fancy stuff like re-running just the tests for the code that has changed.

I use and know many who use CRA successfully for enterprise (massive) applications without the need to eject.

Not yet for large apps, we use it for small standalone static frontends (for tools used internally).

We'd like to reuse TypeScript modules in CRA-based apps, while keeping React+SASS+CSS modules, which isn't possible as of now (we use custom-react-scripts-with-modules which is outdated and not maintained anymore by its author who replaced it with customizable-react-scripts which is a webpack-config-based solution).

Please do not take this as an attack, but as a genuine interest in what we could do better or enable via plugins.

Sure, you're very welcome.

We're currently in the works to support TypeScript & CSS Modules, which are two largely requested features.

Really promising, thanks!

@sompylasar
Copy link

Looks like https://neutrino.js.org/ is awesome: easily configurable without diving into webpack configs and has multitude of presets compatible with each other. Thanks again @Timer. I think CRA has just lost me as a customer, I really need to give Neutrino a try.

@Timer
Copy link
Contributor Author

Timer commented Jul 15, 2017

Wow @sompylasar! Thanks for the extensive feedback, I really appreciate it.

I didn't know about neutrino, will have a look. How CRA is different from it then, besides it's by Facebook, not Mozilla?

It seems you've already dug into this more and discovered what neutrino is about. 😄

Here are the features that took most of the time (in order of appearance):
...
Useful things we added to the developer experience:
...

Thanks for such an exhaustive list! Some of these things CRA already does, but others sound like awesome features which could make improvements. I'm going to keep these noted. 😄

Looks like https://neutrino.js.org/ is awesome: easily configurable without diving into webpack configs and has multitude of presets compatible with each other. Thanks again @Timer. I think CRA has just lost me as a customer, I really need to give Neutrino a try.

That's fine, it's been nice to have you! neutrino sounds like it will solve your desires much better than we can.

@cr101
Copy link
Contributor

cr101 commented Jul 23, 2017

Any chance that there will be a plugin to enable server-side rendering support?

@markspolakovs
Copy link

markspolakovs commented Aug 9, 2017

Apologies @Timer, but I don't quite understand why you're adamant about plugins being whitelisted by the CRA team, instead of being configurable by end-user developers. If a user is specifically adding a plugin to some kind of configuration (for example, package.json), they're kind of implicitly recognising that they're doing something extra, beyond what CRA itself supports.

Maybe make some kind of flag to enable non-whitelisted plugins (enableNonWhitelistedPluginsDoNotUseOrYouWillBeFired), or have a big box pop up on first run (WARNING! You are using non-whitelisted CRA plugins! These are NOT supported by the CRA team! Don't report issues to us unless you can reproduce without the plugins!), but I think there's a great many people who want to use extra features but don't want to eject for one reason or another.

Personally, I have seen many libraries that I have passed on because they require changes to Webpack configuration, and I don't want to eject. For instance, react-toolbox requires CSS modules, which requires Webpack config changes. If they would create a cra-plugin-react-toolbox (or even a more generic cra-plugin-css-modules), I'd have gladly used react-toolbox. However, based on your own heuristics ("most often requested to be supported by CRA"), it's unlikely the plugin would be whitelisted.

Ninja edit: in addition, this could easily lead to accusations of favouritism: for instance, if you were to not approve some Microsoft or Google plugin (for a perfectly valid technical reason), people could easily see that as Facebook attacking Google, no matter how many times you state that it's not "political" but purely technical. Or, some company could create a plugin but refuse to allow it to be whitelisted because "We have control to modify & publish updates to the package" seems like giving Facebook control (cf. React license patents grant debacle). Both React and CRA are Facebook projects, and whatever you guys do, it will be seen as a Facebook action, positive or not.

@Timer
Copy link
Contributor Author

Timer commented Aug 9, 2017

@markspolakovs

If a user is specifically adding a plugin to some kind of configuration (for example, package.json), they're kind of implicitly recognising that they're doing something extra, beyond what CRA itself supports.

So that's why these plugins are white listed.
Plugins which may be added to your installation are fully supported by CRA, and are expected to be as seamless as a normal installation.
We will never permit a plugin which is not part of this repository, so perhaps plugins is a poor name. It'll be more like an add-on -- an official add-on.

These are feature we want to support, but do not because we'd like to not bloat installations for individuals who do not use them.

Maybe make some kind of flag to enable non-whitelisted plugins.

There can be a fork which allows users to use non-whitelisted plugins, but not the main repository.

Ninja edit

So, I believe this confusion is caused by the name "plugins"; we will never permit "3rd party plugins" because they will break the user experience.

IMO, there will be no more "favouritism" than there is now. We currently disallow a number of suggestions which enable language features, libraries, etc.
Right now, we do not support TypeScript (for example) -- many might think it's because we want to push Flow, but it's strictly a matter of experience.
That's why this plugin system is being explored to support the usage of TypeScript (and friends, e.g. relay). We're not even entirely sure if we'd like to support TypeScript yet -- this is a proposal exploring that.


IMO, if users are too up in arms about these issues at the end of the day we will probably forego any plugin/addon system and never support things like TypeScript or Relay.
This is an excellent opportunity for forks to fill the voids.

@agos
Copy link

agos commented Aug 10, 2017

@markspolakovs I think that if the proposed extensibility system will go through (which I hope it will!) soon enough a fork enabling non whitelisted add-ons will appear.

While this might not seem an improvement over the current situation (i.e. having to use a fork), the benefits would be many:

  • the fork would be a lot easier to keep up to date
  • the CRA team would still benefit from the reduced feature request load 😄 while not needing to support add-ons which they might not even know are out there
  • the people needing a custom add-on would have a very simple way to use them, while reducing the worry of falling behind the latest CRA updates
  • the people not needing a custom add-on would enjoy the added choice from the whitelisted add-ons

I think the proposed system, with the whitelisting, is an excellent idea - the only thing I would humbly suggest is not to call it a plug-in system, because I fear the word alone could cause expectations.

@markspolakovs
Copy link

@agos @Timer Fair enough. This fork is going to be inevitable, but it seems like the way forward - having to use an unofficial fork cements in your mind the unofficial-ness of it all.

@lifeiscontent
Copy link
Contributor

lifeiscontent commented Aug 13, 2017

@Timer any idea when this feature might be finished?

@joshmanders
Copy link

I was excited when I saw this, because I love CRA for the whole fact I can get started without messing with the build setup... I also know Webpack and that very well already, so I'd prefer not to eject for the fact that I'd have to learn all the ways CRA team set up webpack. If I had to eject I'd just go the route of manually setting everything up myself without CRA.

The idea of plugins sounded great because then I can use stuff that isn't officially supported but could be community supported... But making it CRA team whitelisted only sounds horrible. :/

@bootstraponline
Copy link

I think any plugin system, whitelisted or not, is a significant improvement.

@lnhrdt
Copy link

lnhrdt commented Nov 7, 2017

It's worth noting that react-app-rewired could be seen as a poor man's plugin system, and the community growing around it could be seen as validation that the community needs an extensible system. Even as a contributor of a few of the community maintained "rewires" I'd much prefer to see a first-class plugin system in create-react-app as I suspect it would be much easier to maintain a stable ecosystem of extensions.

I'd also add that react-app-rewired offers a compelling example of a stable, open system so I'd advocate against the whitelist proposal.

CC: @timarney (the react-app-rewired maintainer)

@AhmadMayo
Copy link

AhmadMayo commented Jan 22, 2018

@Timer I'm a heavy user of CRA, so first I would like to thank you for the great tool, and I really appreciate the effort you put into this project.
Second, a couple of ideas as a small contribution:
1- Don't bother supervising or censoring the plugins, just let the community do it's work. Good popular plugins will flourish, bad plugins will die. React has a huge ecosystem, and nobody ever thought of whitelisting its libraries
2- As different plugins will require different handling according to the tool they address (jest/babel/webpack) why not make it part of the name? i.e. react-scripts-jest-plugin-* etc. They will have long names, but what the heck? we're developers, long descriptive names are better than elegant short ambiguous ones, right?

@bootstraponline
Copy link

What's blocking this PR from being merged?

@danawoodman
Copy link

Is the progress on this stalled? Some ability to extend CRA would be very desirable. Just a friendly ping to see how its going!

@timarney
Copy link

timarney commented Aug 9, 2018

#670 (comment)

@danawoodman
Copy link

If that’s the case then shouldn’t this be closed?

@Timer Timer closed this Aug 9, 2018
@lock lock bot locked and limited conversation to collaborators Jan 19, 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.