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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elm 0.19.1 #13

Merged
merged 4 commits into from Nov 4, 2019

Conversation

@brandly
Copy link
Contributor

brandly commented Nov 3, 2019

hello again!

i upgraded this app to the 0.19.1 elm compiler. the only thing that needed changing is removing the port declaration. the new compiler gave clear feedback:

This module does not declare any ports, but it says it will:

1| port module Generators.ElmDep exposing (Model, Msg(..), StateVizData(..), finishedDownloadingWith, getPathsOfElmFiles, initialModel, stateVizData, toGraphFile, update)
   ^^^^^^^^^^^
Switch this to module and you should be all set!

it's also mentioned in the upgrade instructions.


this pull request consists of two commits. the second adds a package.json, declaring the specific elm binary that's used. with this, users just npm install and have a local dev environment, without having to install anything globally. this is great when working on multiple projects that use different versions of elm.

i modified the Makefile to use the locally-installed elm. the package.json also lists make as its postinstall script. upon cloning this repo, npm install downloads an elm binary and makes the project. a full setup in a single command.


@erkal let me know what you think. if you just want the first commit, we can do that, but maybe you agree about the other changes 馃槃

otherwise, i can also update the readme with the new development instructions! thanks.

@erkal

This comment has been minimized.

Copy link
Owner

erkal commented Nov 4, 2019

Great. Thank you!
I was thinking about using parceljs.
What do you think about it?

@brandly

This comment has been minimized.

Copy link
Contributor Author

brandly commented Nov 4, 2019

@erkal I鈥檝e used parcel on JS projects and enjoyed it. I haven't used it with Elm. What are you hoping to gain from it?

I鈥檝e been using a setup like this pull request on Elm projects lately. The elm compiler on its own is very fast. Here鈥檚 a project that has a script that produces a minified bundle and pushes it to gh pages. https://github.com/brandly/elm-dr-mario/blob/master/package.json

@erkal

This comment has been minimized.

Copy link
Owner

erkal commented Nov 4, 2019

This looks very nice.
Having something like this, we would also to get rid of makefile and the deploy script.

@brandly

This comment has been minimized.

Copy link
Contributor Author

brandly commented Nov 4, 2019

^ the setup i linked to has been nice. the elm binary is very fast, like 0.05s, but running npm run build puts it up to like 0.60s. the extra time is spent initializing a JS runtime, i think. i was considering moving to a Makefile for this reason haha

i don't have any experience using parcel with Elm. having it watch files and do incremental builds on JS projects is nice, since JS builds tend to be very slow by comparison.

looking at this, it appears you point it at an html file, which is kinda nice. however, that html references a JS file, so maybe the builds are slower and more involved due to the surrounding JS. i don't really know.

it's your decision though!

@erkal

This comment has been minimized.

Copy link
Owner

erkal commented Nov 4, 2019

Ok, after all the conversation, I've found the way in your pull request the best.
I didn't know that npm adds so much time.

@erkal

This comment has been minimized.

Copy link
Owner

erkal commented Nov 4, 2019

If you update the README, I'll merge it to master.

brandly added 2 commits Nov 4, 2019
@brandly

This comment has been minimized.

Copy link
Contributor Author

brandly commented Nov 4, 2019

thanks for discussing everything!

@erkal

This comment has been minimized.

Copy link
Owner

erkal commented Nov 4, 2019

thank you for taking the effort to explain.
it saved plenty of time for me.
鉂わ笍

@erkal erkal merged commit 21db19f into erkal:master Nov 4, 2019
@brandly brandly deleted the brandly:elm19.1 branch Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.