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

Delay copying of elm files to /gen folder until cli app is done #23

Merged
merged 3 commits into from
Jan 4, 2020

Conversation

danmarcab
Copy link
Contributor

Before this change webpack started compiling before the cli version
if the app was compiled in doCliStuff.

Not sure how but this was causing webpack to send the cli version
of the app to the browser when main.js was requested. This caused
the app to not render anything (because it was a worker app) and
not show any errors.

The change just waits until the cli finished compiling to only
then move the .elm files to the /gen folder, triggering webpack
to recompile using the correct (ui version) of the app.

This is to fix: #22

@dillonkearns I am sure there is a better solution, but I just couldn't take it anymore 😄 . I am happy to use my fork in the meantime while you work out a better solution, so feel free to close in that case 👍 .

Thank you again for your work!

Before this change webpack started compiling before the cli version
if the app was compiled in doCliStuff.

Not sure how but this was causing webpack to send the cli version
of the app to the browser when main.js was requested. This caused
the app to not render anything (because it was a worker app) and
not show any errors.

The change just waits until the cli finished compiling to only
then move the .elm files to the /gen folder, triggering webpack
to recompile using the correct (ui version) of the app.
@dillonkearns
Copy link
Owner

Hello Dan! Thank you so much for this 🙏

Sorry for the delay, I was on my wedding trip when you made this PR, and I just got back from my honeymoon! So it's been a bit of a crazy couple of months.

I took a look and it seems to work really smoothly! I'll need to investigate a bit more to see what's going on and what I can learn from your changes to keep improving this. I should be merging this in shortly.

Thanks again!

@dillonkearns dillonkearns merged commit bf603a9 into dillonkearns:master Jan 4, 2020
@dillonkearns
Copy link
Owner

@all-contributors please add @danmarcab for code

@allcontributors
Copy link
Contributor

@dillonkearns

I've put up a pull request to add @danmarcab! 🎉

@dillonkearns
Copy link
Owner

I merged in the latest master and pushed a fix! https://github.com/dillonkearns/elm-pages/blob/master/CHANGELOG-NPM.md#115---2020-01-03

Thank you so much Dan! And great work on investigating this and understanding the underlying issue. I really appreciate your help here!

I tried to get hot reloading working initially on this project, but all my fiddling seemed to have no effect. Maybe now it's worth doing some more fiddling there. I think I'll revisit that, that would be a killer feature.

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.

Content edits are not reloaded in dev mode
2 participants