search

marko-js-archive / marko-loader

DEPRECATED: see https://github.com/marko-js/webpack
MIT License
10 stars 12 forks source link

Take webpackOptions.module.rules into account for loaders (webpack 2.x compatibility) #5

Closed jrop closed 7 years ago

jrop commented 7 years ago

Take webpackOptions.module.rules into account for loaders when using webpack 2.x

As of webpack 2.x, loaders are now specified as rules: When an inline style is used with less (style.less {...}) The marko-loader used to only search through options.module.loaders for the less-loader. In order to fix this, and make the fix work for previous versions of webpack (viz, 1.x), webpack/lib/ResultSet.js has been copied into this module. Fixes #4

Pulling in webpack/lib/ResultSet.js seems a little messy. An alternative to this would be to publish a breaking change of this package, and just import it from webpack instead (require('webpack/lib/ResultSet')). However, including it in this project is a means of backporting compatibility to webpack 1.0.

mlrawlings commented 7 years ago

Thanks @jrop for the PR! I'm not thrilled about copying in a webpack file, but it might be our best option.

One more option: this package could directly depend on webpack@^2 and the top level application could still use 1.x it just might be that 2 different versions of webpack end up getting installed.

I do have a concern with any option that uses require('webpack/lib/ResultSet') because it doesn't look to be documented anywhere, so if it's considered private, that API could change without warning.

Otherwise this looks great, any thoughts before we merge this?

//cc @patrick-steele-idem

jrop commented 7 years ago

@mlrawlings These are valid concerns.

One more option: this package could directly depend on webpack@^2 [...]

My thought on this, and why I opted to import the file, draws on your second concern:

[...] it doesn't look to be documented anywhere [...]

Since it is most likely a private API, I think it would be more dangerous to depend on webpack@^2, since updates to webpack could break the API, as you mentioned, which is mitigated by copying the file into this repo. As we know the version copied behaves as expeced, and provides the expected functionality. Also, pulling in webpack as a dependency would pull in a ridiculous amount of dependencies (~300).

One other option would be to develop logic that can parse webpack rulesets. The benefit of this would be that (most likely), the developed solution would be more lean than the file I pulled in. The obvious downside to this is that it is another piece of logic to maintain. I would prefer this solution if I had extra time (perhaps in the future if I have extra time and feel up to it, I could create another PR exploring this option).

austinkelleher commented 7 years ago

@jrop Thanks for the PR. module.rules should now be accounted for in https://github.com/marko-js/marko-loader/pull/14. Please let me know if you still see any issues.