-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
Using the new getOptions ensures we avoid the TypeError for loaderUtils.getLoaderConfig not being a function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you try fixing Travis builds? https://travis-ci.org/NoRedInk/elm-assets-loader/builds/219670239
(Thanks for making a PR & sorry not getting around to it sooner)
@avdgaag any chance you could resolve the build issues? Be awesome to get this merged. |
@TimPerry Resolving the build issues would involve upgrading to Webpack 2, triggering more or less a rewrite of all the tests (since they create invalid Webpack 2 configurations) and of course an update of documentation/setup instructions. This is not an enormous amount of work, but it’s not trivial either. I might get around to giving it a shot it over the weekend, but I can’t make any promises. |
I took a stab at fixing this, but I don’t think it’s something I can solve. Updating the Webpack configuration in the tests to be Webpack 2 compatible just led to a whole new rabbit hole of failed tests due to changes in how Webpack 2 works. In my opinion, the entire test suite needs to be rewritten around Webpack 2 and I don’t have the understanding of what we’re trying to test nor of webpack internals to do that 😞 |
Taking a stab at fixing tests |
Interesting to see this take shape… 😉 |
Travis is green for both webpack 1 and 2. Merging! |
Published as 0.3.0. Let me know if it doesn't work in any way |
Nice! I think the changes in 3e8c664 are what tripped me up. I’ll check if everything still works and report back if it doesn’t. Thanks for fixing this! |
Using the new getOptions ensures we avoid the TypeError for loaderUtils.getLoaderConfig not being a function.