Vjp/use spago #122
Vjp/use spago #122
Conversation
Manually adds packages missing from the main package set in packages.dhall
Also added a "setup" script. Because i've been bitten many times by forgetting that "css" is a separate npm project which needs dependencies installed
|
Ah wait, I think I omitted RTS flags. Let me try that and see if it speeds up |
Spago always does a dependency installation check prior to attempting a build
spago doesn't forward RTS args to purs compile when using `spago bundle-app`, so we bypass spagos bundle and use purs directly
package.json
Outdated
| "postinstall": "", | ||
| "setup": "npm i && cd css && npm i", |
There was a problem hiding this comment.
This isn't a special npm command similar to postinstall is it? If this is simply intended as an alternative to running npm install, then maybe we can use postinstall instead? Something like:
| "postinstall": "", | |
| "setup": "npm i && cd css && npm i", | |
| "postinstall": "cd css && npm install", |
That way we don't have to remember this different command to run, as that wouldn't be much different as having to remember to install CSS dependencies.
As an aside, I'll try to find some time to move us to a better build system such as make so we wouldn't have to concern ourselves with whether dependencies are installed or not.
There was a problem hiding this comment.
No theres nothing special about setup, its just a standard script. The problem with putting it in postInstall is it will cause the `install node/spago dependencies" to run longer even though there's the dependencies will already have been added.
Maybe a better alternative is to add everything in the setup script to the clean script. That way clean won't actually clobber out all the npm packages. What do you think? @davezuch
There was a problem hiding this comment.
The problem with putting it in
postInstallis it will cause the `install node/spago dependencies" to run longer even though there's the dependencies will already have been added.
I'm not sure I follow. Are you worried that running npm install will take longer because now that will also include installing css dependencies when they've already been added? If they've already been added, then there won't be anything to install, so it shouldn't add much time.
Or are you worried about builds such as in Circle CI where the css dependencies aren't needed to begin with? In that case, I don't think it's a huge sacrifice, but if we can move to a better build setup as I talked about above, it also wouldn't be a concern.
There was a problem hiding this comment.
Haha the latter - didn't want to increase the build-time on circle ci any further than it already has. If that doesn't seem like an issue, I can just throw it in postinstall. A better build system will help in the future anyhow
There was a problem hiding this comment.
Got it, let's throw it in for now and see if the increased build time is worth the savings in not having to remember to install the css separately. At least from the perspective that we'll improve the build system soon.
Its causing circle-ci to get caught in some sort of infinite install loop
package.json
Outdated
| @@ -4,7 +4,7 @@ | |||
| "private": true, | |||
| "scripts": { | |||
| "build-all": "yarn run build-ui && yarn run build-css", | |||
| "postinstall": "npm i && cd css && npm i", | |||
| "postinstall": "", | |||
There was a problem hiding this comment.
The infinite loop was caused by the first npm i && . postinstall only runs after npm i is called so running it from a postinstall will result in an infinite loop.
With a postinstall of "cd css && npm i", you basically end up with:
alias npm i="npm i && cd css && npm i"With a postinstall of just "npm i", you'd end up with something like:
alias npm i="npm i && npm i && npm i && npm i && npm && ..."I'm not sure if my example is clear, but
package.json
Outdated
| "postinstall": "", | ||
| "setup": "npm i && cd css && npm i", |
There was a problem hiding this comment.
Got it, let's throw it in for now and see if the increased build time is worth the savings in not having to remember to install the css separately. At least from the perspective that we'll improve the build system soon.
What does this pull request do?
Migrate ocelot away from bower to spago
@davezuch Im really not sure if I actually setup all the build scripts correctly or not. Let me know if theres something I need to change. The build portion is taking a pretty long time - 4:35 seconds when the build has already been cached. Im not sure why its taking so long. Spago outputs "build succeeded" and then proceeds to sit there silently for a long time. I guess the bundling is taking a really long time?
Alright after doing a bit of research, apparently spago doesn't pass rts_args to purs bundle correctly. So I bypassed it manually - the build time has sped up significantly but its still a bit slow all in all. 2 minutes is better than 6 though.
purescript/spago#580
purescript/spago#216
Let me know if this all looks good or if anything needs to be changed!