search

madsflensted / elm-brunch

Brunch plugin to compile Elm code
MIT License
74 stars 31 forks source link

Make compilation in onCompile hook instead of compile method #30

Closed stelmakh closed 7 years ago

stelmakh commented 7 years ago

There were some reports about brunch missing changes in elm files after compilation error occurred. Here's the original StackOverflow question.

As I've mentioned on the StackOverflow

This is both brunch and elm-brunch issue. Brunch plugins are designed to compile each file separately on change. elm-brunch, however, runs elm-make for elm modules instead. That's why brunch cache is not being updated properly, causing redundant error messages.

Running the compilation in the onCompile hook instead of the compile method is more suitable in this case.

madsflensted commented 7 years ago

@stelmakh thank you for the PR. I have to small requests:

You have mixed refactoring (I expect that it is all good stuff) with the new feature making it hard to review the important change at a glance.

The tests no longer pass.

stelmakh commented 7 years ago

@madsflensted Thanks for the reply. My bad, will fix it as soon as possible.

stelmakh commented 7 years ago

Tests has been fixed. As for new feature/refactoring mixing, the only thing that is new is that all the logic is now moved to the onCompile hook instead of the compile method. This allows brunch to update it's cache properly and still be able to compile Elm files. Apart from that change, everything else is the refactoring.

madsflensted commented 7 years ago

Thank you for the update. Sorry about the late merge. Will publish new version.

stelmakh commented 7 years ago

Great, thanks a lot!

wpiekutowski commented 7 years ago

It looks like this PR is changing the order of compilation.

With 0.7.0 Elm files were compiled early, before everything else, so that the resulting .js files could have been picked up by brunch and concatenated in one go.

With 0.8.0 Elm files are compiled at the very end, after concatenation is done. This still works with brunch watch, because brunch will simply run for the second time and pick up resulting .js files. Unfortunately it breaks deployment scripts. They run brunch only once, so Elm output is missing. A workaround is to commit resulting .js file, but that's an ugly solution. Was this intended?

dustinfarris commented 7 years ago

Yeah i'm also noticing what @wpiekutowski mentioned. having elm run after compile bypasses any other js-related brunch plugins you might be using.