CB-349: Gulp to webpack #253
Conversation
There was a problem hiding this comment.
Good job moving the project over to webpack !
I wasn't able to see it in action using the docker-compose -f docker/docker-compose.dev.yml up -d command.
Looking at the Dockerfile.dev it doesn't look like you're runnin npm run build anywhere.
The CB docs talk about running a command in a new container, which seems a bit cumbersome.
Do you know what the reason is not to compile the static assets on build in dev environment, or is that a question for @alastair ?
We can't do this during development because we mount the working directory into the web container as a volume, and this overrides any commands that we run during build. This is the same way that we do static assets for build in AB |
|
OK, thanks @alastair . Personally I don't mind running npm commands on my machine. |
|
@MonkeyDo Thanks for suggesting improvements. I've made the changes. |
|
Look good, nice job ! |
bimalkant-lauhny
changed the title
bimalkant-lauhny
changed the title
|
New changes:
|
|
In LB, We do live compiling by adding a seperate container that has webpack build --watch running inside and the static assets are moved out of the volume. I'd like a similar setup here. |
|
@paramsingh please review now! I have added the changes. |
There was a problem hiding this comment.
The new setup doesn't seem to be working for me.
I get a 404 on js and css resources: GET http://localhost:8080/static/common.js net::ERR_ABORTED 404 This should be static/build/common.js, which loads correctly.
Issue with the static_manager or the manifest file, perhaps?
| @@ -0,0 +1,8 @@ | |||
| FROM node:10.15-alpine | |||
There was a problem hiding this comment.
Does the builder have to be a separate docker image? Typically we try to share our images as much as possible. The Dockerfile for CB also performs npm install, so as far as I can tell we should be able to build the static files using a container based on that image
There was a problem hiding this comment.
@alastair Well, I just tried to take inspiration from the new setup in LB. 😅 @paramsingh @MonkeyDo what do you guys think about this?
There was a problem hiding this comment.
alastair is absolutely right! If the CB docker image can run npm commands, then there's no need to use another image.
For LB, there was previously nothing using node so the image did not contain node.
admin/compile_resources.sh
Outdated
| @@ -1,4 +1,4 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
I don't see this file used anywhere in this repository. Is it needed?
Issue with the flask server watching the manifest.json file
|
@code-master5 : After discussing it with alastair, removing CleanWebpackPlugin and running the asset building once before starting the services does the trick (at least until a better solution is found). I've added some commits to document that. |
|
@paramsingh @ferbncode please find some time to review and merge this (or suggest changes if any). |
|
@ferbncode @paramsingh Any reasons for not reviewing or merging this PR? |
|
Thanks, merged into master! |
|
Thanks @alastair ! 🙇 |
Summary of changes:
package.jsonwith packages to support Webpack, replacednpm-shrinkwrap.jsonwithpackage-lock.json.gulpand addedwebpack. Added plugins to convertlesstocss, clean build directory and generatemanifestfile.static_manager.pyto read from newermanifestfile..babelrcwith newer babel env.nodeto version10.xin Dockerfile. Also updated command to build scripts and styles.