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.

Sign up for GitHub

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

CB-349: Gulp to webpack #253

Merged
merged 18 commits into from
Aug 3, 2020
Merged

CB-349: Gulp to webpack #253

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2ab9a29
Updated package.json, replace npm-shrinkwrap with package-lock
bimalkant-lauhny Mar 18, 2019
adefe89
Update babelrc
bimalkant-lauhny Mar 18, 2019
9e79bd3
Added webpack config
bimalkant-lauhny Mar 18, 2019
59ba8fa
Remove gulpfile
bimalkant-lauhny Mar 18, 2019
83261f7
Update static manager to read manifest file in build dir
bimalkant-lauhny Mar 18, 2019
c7d09c3
Update to Node 10.x and build command in Dockerfile and compile script
bimalkant-lauhny Mar 18, 2019
66446d6
Add less file to script for webpack compilation
bimalkant-lauhny Mar 18, 2019
6938ff2
Update webpack to use contenthash
bimalkant-lauhny Mar 21, 2019
45a66cf
Correct import main and leaflet less, images path
bimalkant-lauhny Mar 25, 2019
c7dc74d
Remove webpack-dev-server from package.json
bimalkant-lauhny Mar 25, 2019
a863556
Remove unnecessary single chunk optimization from webpack config
bimalkant-lauhny Mar 25, 2019
6419b12
Add separate container for static files
bimalkant-lauhny Apr 8, 2019
03b8975
Add support for dev mode
bimalkant-lauhny Apr 8, 2019
7f5ba7c
Use contenthash for prod only
bimalkant-lauhny Apr 10, 2019
5cc5ddf
Observe changes in manifest.json to reload flask server
bimalkant-lauhny Apr 10, 2019
9c4d4e3
Remove unnecessary docker volume and script, updated webpack config
bimalkant-lauhny Apr 25, 2019
9dc523c
Removed CleanWebpackPlugin
MonkeyDo Apr 30, 2019
7ea9cb5
Added and documented step to build static assets
MonkeyDo Apr 30, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .babelrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
{
"presets": ["es2015", "stage-2"],
"presets": [
"@babel/preset-env"
]
}
5 changes: 2 additions & 3 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ RUN apt-get update \
ENV PGPASSWORD "critiquebrainz"

# Node
RUN curl -sL https://deb.nodesource.com/setup_6.x | bash -
RUN curl -sL https://deb.nodesource.com/setup_10.x | bash -
RUN apt-get install -y nodejs

RUN pip install uWSGI==2.0.13.1
Expand All @@ -41,13 +41,12 @@ RUN pip install -r requirements.txt

# Node dependencies
COPY ./package.json /code/
COPY ./npm-shrinkwrap.json /code/
RUN npm install

COPY . /code/

# Build static files
RUN ./node_modules/.bin/gulp
RUN npm run build

# Compile translations
RUN pybabel compile -d critiquebrainz/frontend/translations
Expand Down
8 changes: 8 additions & 0 deletions Dockerfile.webpack
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
FROM node:10.15-alpine
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alastair Well, I just tried to take inspiration from the new setup in LB. 😅 @paramsingh @MonkeyDo what do you guys think about this?

Copy link
Member

@MonkeyDo MonkeyDo Apr 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


RUN mkdir /code
WORKDIR /code

COPY package.json package-lock.json webpack.config.js /code/
RUN npm install

2 changes: 1 addition & 1 deletion admin/compile_resources.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this file used anywhere in this repository. Is it needed?


cd "$(dirname $0)/../"
node_modules/.bin/gulp $@
npm run build $@
121 changes: 0 additions & 121 deletions critiquebrainz/frontend/static/gulpfile.js
View file
Open in desktop

This file was deleted.

3 changes: 2 additions & 1 deletion critiquebrainz/frontend/static/scripts/leaflet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
import '../styles/leaflet.less';
var L = require('leaflet');
L.Icon.Default.imagePath = '/static/images';
L.Icon.Default.imagePath = '/static/images/';
1 change: 1 addition & 0 deletions critiquebrainz/frontend/static/scripts/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '../styles/main.less';
3 changes: 2 additions & 1 deletion critiquebrainz/frontend/static_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os.path
import json

MANIFEST_PATH = os.path.join(os.path.dirname(__file__), "static", "build", "rev-manifest.json")
MANIFEST_PATH = os.path.join(os.path.dirname(__file__), "static", "build", "manifest.json")

manifest_content = {}

Expand All @@ -14,6 +14,7 @@ def read_manifest():


def get_static_path(resource_name):

if resource_name not in manifest_content:
return "/static/%s" % resource_name
return "/static/build/%s" % manifest_content[resource_name]
10 changes: 10 additions & 0 deletions docker/docker-compose.dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ services:
- db_test
- critiquebrainz_redis
- musicbrainz_db
- static_builder

critiquebrainz_redis:
image: redis:4.0-alpine
Expand All @@ -50,3 +51,12 @@ services:
MB_IMPORT_DUMPS: "true"
ports:
- "5430:5432"

static_builder:
build:
context: ..
dockerfile: Dockerfile.webpack
command: npm run dev
volumes:
- ..:/code:z
- ../critiquebrainz/frontend/static:/static:z
MonkeyDo marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 0 additions & 2 deletions gulpfile.js
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion manage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
# Files listed here will be monitored for changes in debug mode and will
# force a reload when modified.
OBSERVE_FILES = [
"critiquebrainz/frontend/static/build/rev-manifest.json",
"critiquebrainz/frontend/static/build/manifest.json",
]


Expand Down
Loading