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

Configure plugin order in config rather than rely on package.json order #1377

Open
facboy opened this issue May 26, 2016 · 35 comments
Open

Configure plugin order in config rather than rely on package.json order #1377

facboy opened this issue May 26, 2016 · 35 comments
Labels
Milestone

Comments

@facboy
Copy link
Contributor

@facboy facboy commented May 26, 2016

It seems a bit fragile to have to rely on the order of declaration in package.json to determine the order that plugins are invoked in, given that npm --save reorders the dependencies alphabetically. I'm basing this on brunch-guide, and a cursory glance over the source code.

It would be nice if this could be overridden in config.

On another note, I'd quite like to be able to add locally-defined plugins without having them be a package in node_modules.

@paulmillr
Copy link
Member

@paulmillr paulmillr commented May 27, 2016

Agree. We should expose something like plugins: order.

As for the locally-defined plugins, our goal here is to not introduce new APIs that duplicate current ones. Maybe there's a way of loading your local package while still using package.json

@goshakkk
Copy link
Contributor

@goshakkk goshakkk commented May 27, 2016

Ok, manual ordering can make sense.

Could you expand on locally defined plugins? If you mean you want your project to both contain a plugin and use that, I can't see any problems with that. npm has a way to use packages from the file system:

{
  "devDependencies": {
    "my-super-plugin": "file:./my-super-plugin"
  }
}
@facboy
Copy link
Contributor Author

@facboy facboy commented May 27, 2016

Yeah, I'm currently using that npm mechanism, but if I'm editing the plugin I have to continually do an npm install to keep it up to date with the file system. I could also npm link it.

However, defining a separate package, while trivial, seems a bit overkill when I could just use a locally defined (eg in brunch-config) class/object and inject it into the config.

@goshakkk
Copy link
Contributor

@goshakkk goshakkk commented May 27, 2016

Well the use case we cover is editing your project and seeing the changes — not live-developing plugins. It does seem like a very niche one, and I'm not sure we want to complicate the internals and expose even more config for something that'll be barely used. cc @paulmillr?

@hgwood
Copy link

@hgwood hgwood commented Jun 15, 2016

I think both the features presented in this issue would be very useful. Maybe something like this in the config?

plugins: {
  order: ['plugin1', 'plugin2'],
  locals: {
    plugin2: require('./plugin2') // ./plugin2.js exports a normal plugin class
  }
}
@michaelhogg
Copy link

@michaelhogg michaelhogg commented Oct 9, 2016

At the very least, the main documentation about plugins needs to state that plugins are executed in the order specified in package.json.

(I found a mention of this here in the Brunch.io Guide, but it needs to be stated clearly in the main documentation on brunch.io.)

This seems extremely unintuitive, and I don't think I've encountered any other situation where the order of dependencies in package.json has any consequence.

In addition, regarding the dependencies and devDependencies objects in package.json, the JSON spec states:

An object is an unordered set of name/value pairs.

As mentioned in this Stack Overflow answer: "You cannot and should not rely on the ordering of elements within a JSON object."


For the benefit of people arriving here from a search engine:

brunch build was giving me a compilation error for a JS source file which exported an ES6 module:

error: Compiling of app/module.js failed. Line 5: Unexpected token

I found this error was being generated by Esprima on this line in javascript-brunch.

After investigating, I found the pipeline was using this array of compilers: [JavaScriptCompiler, BabelCompiler].

After further investigation, I found the ordering of the compilers was originating in loadPackages(), which loads the list of dependencies from package.json.

In my package.json, I had javascript-brunch before babel-brunch. So Esprima was trying to parse the JS source file before Babel had transpiled it 😕

@shvaikalesh
Copy link
Contributor

@shvaikalesh shvaikalesh commented Oct 12, 2016

Hey @michaelhogg, thanks for such detailed post. After #1522 is merged, you can use plugins.order to configure order of plugin execution:

plugins: {
  order: [
    'babel-brunch',
    'javascript-brunch',
  ],
},

Also, I have added a note to Brunch docs on the website. We would love to hear more thoughts from you on how to improve Brunch in the future. Thank you.

@michaelhogg
Copy link

@michaelhogg michaelhogg commented Oct 12, 2016

@shvaikalesh: Awesome, thank you so much! 😀

I'm really enjoying using Brunch, and I've made a few contributions to eslint-brunch. I'll definitely continue to share contributions/feedback where I think things can be improved. Many thanks to the whole Brunch team for building such a great tool!

@shvaikalesh
Copy link
Contributor

@shvaikalesh shvaikalesh commented Oct 17, 2016

@michaelhogg After some thoughts and discussion, we've decided to eliminate the need of plugins.order. Instead we are going to drop javascript-brunch and css-brunch in the near future so the ordering detail you've encountered will never be a problem. We are aiming to make Brunch more transparent for users, but these plugins are extra stuff in the package.json we wanna get rid of. Will appreciate your feedback on this. Thanks.

paulmillr added a commit to brunch/brunch.github.io that referenced this issue Oct 17, 2016
@shvaikalesh
Copy link
Contributor

@shvaikalesh shvaikalesh commented Oct 17, 2016

If anyone else has issues with package sorting (i.e. have some optimizers that depend on order and this blocks you from using --save-dev), we will greatly appreciate your feedback. Please feel free to reopen this.

@natecox
Copy link

@natecox natecox commented Oct 19, 2016

Even if javascript-brunch and css-brunch are no longer an issue, the whole point of a system like this is modularity; meaning that we may not actually be using css-brunch (for example, I use postcss-brunch instead) and this fix would not be of value.

I strongly believe that depending on the order of package.json is flawed. Any time you install a package with --save[-dev] you're going to be alphabetizing that list, so you essentially remove from the developer the ability to reliably control the order of execution of their own build tool.

Also, using --save and --save-dev to control the order breaks the fundamental purpose of the package dichotomy. NPM has given us the ability to differentiate between tools that are important to the building of an application locally and libraries that are essential to our production code so that we can optimize our server builds. Requiring the placement of development tools into the production library list is simply a code smell.

The simplest solution here if you're adamantly against having an ordering variable may be to run plugins in the order they are defined in the plugins section of the brunch config. This would allow me as a developer to specify that my postcss optimizations should be run before my Sass transpiler without adding an extra list... although I don't really understand why the list is a bad solution.

@paulmillr
Copy link
Member

@paulmillr paulmillr commented Oct 19, 2016

@natecox we are going to add order field to the plugins themselves instead of forcing users to specify it in config. This would mean package.json order wouldn't matter.

@natecox
Copy link

@natecox natecox commented Oct 19, 2016

How are the plugin authors supposed to understand what their priority should be inside of my build process? I feel like it's a lot to ask of plugin developers, and invites another scenario where developers become blocked by a build step they don't have the power to control.

@paulmillr
Copy link
Member

@paulmillr paulmillr commented Oct 19, 2016

@natecox we'll just use default order for most plugins; higher order for stuff like postcss-brunch. So postcss would be executed after sass compilation.

What is your use case for the different plugin order?

@natecox
Copy link

@natecox natecox commented Oct 19, 2016

Taking the example of postcss and sass integration, there are basically two methods that you could use to join the two:

  1. Sass compilation and then PostCSS compilation again on the result, or
  2. PostCSS optimization of the Sass itself and then compilation.

The second use case is valuable for people currently using Sass for their CSS solution, and PostCSS for optimizations such as auto-prefixing. However, both are valid use cases supported by PostCSS which Brunch itself should not be precluding. By mandating the order of operations you have chosen for the developer which technique they should be using, and this simply isn't a case where convention needs to win out over configuration.

As a case example, the order you proposed above already negates my build preferences because there's no way that you could (or should need to) understand the specifics of it. This is only one example, and I can't imagine that there aren't currently more, or that more won't be created as the list of plugins increases.

@paulmillr
Copy link
Member

@paulmillr paulmillr commented Oct 20, 2016

If it makes sense we can add the ability to switch order later.

But, I don't understand — how can a postcss parser parse sass?

@natecox
Copy link

@natecox natecox commented Oct 20, 2016

@paulmillr paulmillr reopened this Oct 20, 2016
@paulmillr paulmillr added the feature label Oct 20, 2016
@paulmillr paulmillr added this to the 2.10 milestone Oct 24, 2016
@bigardone
Copy link

@bigardone bigardone commented Mar 20, 2017

Hello! I'm also experimenting the same issue. I'm using elm-brunch and babel-brunch, where the main.js file generated by elm-brunch is imported in another js file, but the build fails because it doesn't build Elm first, so the import file is not found.

brunch build
18:26:27 - info: compiling
18:26:31 - info: compiling.
18:26:36 - info: compiling..
18:26:40 - info: compiling...
18:26:44 - error: Processing of web/static/js/views/project/show.js failed. Error: Could not load module '../../main' from '/Users/user/Documents/projects/project/web/static/js/views/book_builder'. Make sure the file actually exists.

I've tried to to move the elm-brunch plugin on top of the npm dependencies, but it doesn't work:

 "devDependencies": {
    "elm-brunch": "^0.8.0",
    "babel-brunch": "^6.0.6",
    "brunch": "^2.10.9",
    ....
}

Is there anything I can do to solve this?
Thanks in advance :)

@natecox
Copy link

@natecox natecox commented Mar 20, 2017

Currently (unless something I'm unaware of has changed), the only thing you can do is manually rearrange your package.json so that the packages show up in the order they need to be run.

Yes, this means that every time you install a new package with yarn add or npm install --save[-dev] you will have to repeat this process because the list will be alphabetized.

@bigardone
Copy link

@bigardone bigardone commented Mar 21, 2017

Hi @natecox, thanks for your response. Unfortunately that doesn't work. As you can see in the package.json I shared previously, elm-brunch is set before babel-brunch, but the result is the same. Babel builds first, throwing the missing file error.

@natecox
Copy link

@natecox natecox commented Mar 21, 2017

You may be able to throw elm-brunch into the standard packages and keep babel-brunch in the development packages; I think I've been able to get some success with this in the past.

The real solution here is, of course, for brunch to stop using package.json as the means for build order determination.

@bigardone
Copy link

@bigardone bigardone commented Mar 21, 2017

@natecox thanks, I'll try that. It would be nice if it could use the current order set in the plugins definition of the brunch-config.js file.
Thanks again for your help :)

@blisscs
Copy link

@blisscs blisscs commented Apr 4, 2017

Hello @bigardone ,

Did you able to solve this. I am facing the same issue here.

Thanks
Dev.

@martin-svk
Copy link

@martin-svk martin-svk commented Apr 4, 2017

Same problem here. Gzip plugin runs before git digest plugin so my gzipped html is different than the not gzipped one.

@mikeprince13
Copy link

@mikeprince13 mikeprince13 commented Apr 29, 2017

Same issue also, with Stylus and PostCSS. I understand the resistance to using plugins.order but as an optional override to the default this does seem like a good solution especially since it mimics files.order.

@shvaikalesh
Copy link
Contributor

@shvaikalesh shvaikalesh commented Apr 29, 2017

@mikeprince13, I am not totally against plugins.order (mimicking files.order is cool too) in config: I see it as a last resort. Stylus + PostCSS issue (and @martin-svk's issue too) is solvable with compiler chaining we will ship quite soon.

PS: really appreciate all you guys commenting this issue and not creating other ones.

@MarianArlt
Copy link

@MarianArlt MarianArlt commented May 6, 2017

@shvaikalesh Looking forward to that, just digging into brunch and though its advertised idea is to make plugins work OOB I'm now into the third day trying to get a simple static site setup work with some preprocessors without fancy modules or anything and it has been somewhat frustrating to be honest. Now I stumble over this thread and I suddenly understand why I got so many errors and wrong outputs...Keep up the efforts it's a nice little program!

Hope this will appear in the documentation once published and just one side note: please make it very clear if ordering is from top to bottom or the other way round. I often find myself not reading clear instructions in some programs when this is critical and can vary.

@polypus74
Copy link

@polypus74 polypus74 commented Nov 29, 2017

Has this been resolved? How do I set the execution order of my plugins, as of today? (Edit: meaning, outside of re-editing package.json after every npm install, which is rather painful)

@natecox
Copy link

@natecox natecox commented Nov 29, 2017

@polypus74 well, the website still indicates that package.json order is used. To my knowledge this is still the case, and is preventing me from seriously adopting Brunch.

@polypus74
Copy link

@polypus74 polypus74 commented Dec 1, 2017

Same. I have no particular attachment to brunch, it just happens to be the default phoenix framework builder. Having to think about whether my project is broken every time I npm install is just not an option, there are already enough things to think about when developing. So unfortunately I am presently researching other possible asset pipelines.

@natecox
Copy link

@natecox natecox commented Dec 1, 2017

@polypus74 I'm also invested into brunch because of Phoenix. I've spent enough time stuck on this particular issue that I've started dropping it and using Webpack instead at the start of every project.

This thread has some decent resources for getting started swapping them out.

@brutus
Copy link

@brutus brutus commented Mar 20, 2018

I have just used Brunch for a few projects, but I started running into this more and more. Any news in which direction this might go?

@natecox
Copy link

@natecox natecox commented Mar 20, 2018

@brutus I haven't seen much development here since this issue opened. I know that Phoenix is in the process of switching from Brunch to Webpack 4 due to inactivity. That shouldn't be taken as proof that this project is "dead", just food for thought.

@thedanbob
Copy link

@thedanbob thedanbob commented Jan 10, 2019

I've been dealing with this issue recently too, trying to get coffee-script-brunch to run before babel-brunch. The best solution I came up with was to rewrite package.json with a postinstall script:

// package.json
"scripts": {
  "postinstall": "perl -0777pi -e 's/([^\\n]+\"babel-brunch\":.+?\\n)(.+\"coffee-script-brunch\":.+?\\n)/\\2\\1/s' package.json"
}

It would obviously be much better to be able to configure this.

@reubano
Copy link
Contributor

@reubano reubano commented May 26, 2020

Would love for this to get fixed. My current situation is the following:

    "terser-brunch": "^4.0.0",
    "@nerevu/cachebust-brunch": "^0.4.2",
    "gzip-brunch": "^1.3.0"

These plugins must be in this specific order or else the build will fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.