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

Make compilation in onCompile hook instead of compile method #30

Merged
merged 2 commits into from
Feb 23, 2017

Conversation

stelmakh
Copy link
Contributor

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
Copy link
Owner

@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
Copy link
Contributor Author

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

@stelmakh
Copy link
Contributor Author

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 madsflensted merged commit 8b062af into madsflensted:master Feb 23, 2017
@madsflensted
Copy link
Owner

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

@stelmakh
Copy link
Contributor Author

Great, thanks a lot!

@wpiekutowski
Copy link

wpiekutowski commented Mar 13, 2017

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants