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

Migrate Grunt to Webpack #192

Closed
wants to merge 20 commits into from
Closed

Conversation

AlinChindea
Copy link

Fix #191

  • replace Grunt with Webpack
  • create Webpack config files for development and production
  • update Travis CI config file

- Installed webpack and webpack-cli as dev-dependencies
- Added a start script in package.json which calls webpack
- Make an index.js fil, which is the file webpack looks for by default
- Webpack spits out our code in dist/main.js by default
- Currently, the main code has nothting to do with webpack, but it will soon
Copy link
Contributor

@katjam katjam left a comment

Choose a reason for hiding this comment

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

I'm assuming this is in progress rather than ready for comments... realised that after I started looking.

@@ -1,3 +1,4 @@
build/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we delete build at the same time - or comment that it is no longer in use?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering why we had just the contents of build before - is it something to do with needing when we checkout to gh-pages or does it seem to be working as it is with ignoring the whole dist dir? Is there a reason to go with dist rather than keep build?

Copy link
Author

Choose a reason for hiding this comment

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

dist is the default production dir that Webpack creates if we do not specify any other name. that will be the be the next step and it will be called build.

src/index.js Outdated
@@ -0,0 +1 @@
alert("hello from webpack!");
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably don't want to keep this.

Copy link
Author

Choose a reason for hiding this comment

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

I'm assuming this is in progress rather than ready for comments... realised that after I started looking.

yeah, my bad, I should have opened it as a draft PR.

</head>

<body>
<h1>Hello from Webpack</h1>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing this is not ready for review yet?

Copy link
Author

Choose a reason for hiding this comment

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

yeah, it's not ready. it's the first step only :)

Alin added 19 commits October 21, 2019 14:38
- Created webpack.config.js
- Added basic configuration, including entry JS file and output dir
- Modified package.json so that webpack user our config file
- Installed style-loader and css-loader
- Configured webpack.config.js to use both loaders on css files
- The order we use these loaders in webpack.config.js matters
- Configured webpack to use ContentHash in bundle file name
- Installed HtmlWebpackPlugin to help us generate a new index.html
- It automatically includes the correct script tag at the bottom
- We passed the handlebars file as a template (including the associated json file)
- Broke our webpack.config file into 3 files
- webpack.common.js, webpack.dev.js, webpack.prod.js
- Installed webpack-merge to share the common functionality
- Updated package.json to use new config files
- Installed webpack-dev-server-and added it to start script in package.json
- Added handlebars-loaders to automatically require the files we reference in img tags
- This doesn't work as of this commit
- Added file-loader to handle svg,png,jpg files from our images
- configured file-loader to add our images to an imgs dir in build
- Also configured it to add a hash to their file names
- Added clean-webpack-plugin to our production config to clean out the build dir each time we build
- use the handlebars-loaders to load the handledbars file into an html file
- use file-loader and copy plugin to copy to build the images we need
- images are now hashed for cache busting
- handlebars template pointing to the right image sources
- still to fix the JS functionality
- Extracted CSS into own file in production
- Minified CSS in production
- Added TerserJS back in to minify JS in production
- Minified HTML in production as well (however, the file is still too large because of the handlebars templates)
- updating jQuery doesn't break anything tested so far
- after testing, the seap-analytics.js needs to be loaded after the scripts.js
- until I figure how to do that with webpack, I commented it out
- created a vendor.js and added a vendor entry into webpack.common
- this should load the seap-analytics after the scripts file
- this solves an error where db would not be defined in the seap-analytics and users cannot see Your assessment section
- Helpers such as accuracy were not properly loaded
- using raw-loader we use the handlebars files as text
- then we recompile it
- hoisted db to window to eliminate reference type errors
- should remove vendor.js file and refactor script.js
@katjam
Copy link
Contributor

katjam commented Jan 13, 2020

@AlinChindea as far as you remember, is this one ready?

@AlinChindea
Copy link
Author

@AlinChindea as far as you remember, is this one ready?

@katjam looking back at the history of my commits it seems tests are not passing, only 2 out of 5. also, I think we need to update the build file.

@katjam
Copy link
Contributor

katjam commented Jan 13, 2020

OK I will close for now.

@katjam katjam closed this Jan 13, 2020
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.

Use Webpack
2 participants