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

webpack implementation #1625

Merged
merged 1 commit into from
Mar 13, 2016
Merged

webpack implementation #1625

merged 1 commit into from
Mar 13, 2016

Conversation

ptrckvzn
Copy link

Here are my changes for the webpack implementation as requested by @retlehs

npm run build build assets
npm run build:production build minimized assets
npm run watch browsersync

dev.js is the dev server with browsersync run by npm run watch (node dev.js)

I'll be happy to keep contributing and get back to you if you have any questions. Looking forward Sage 9!

"build": "bower install && gulp",
"lint": "gulp lint"
"build:production": "export SAGE_ENV=production && webpack",
"build": "export SAGE_ENV=development && webpack",
Copy link
Contributor

Choose a reason for hiding this comment

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

SAGE_ENV=development webpack -d

retlehs added a commit that referenced this pull request Mar 13, 2016
@retlehs retlehs merged commit d1685fe into roots:sage-9 Mar 13, 2016
@retlehs
Copy link
Member

retlehs commented Mar 14, 2016

@ptrckvzn fyi it looks like assets.json got missed

@ptrckvzn
Copy link
Author

assets.json is the file in dist/ generated by the build scripts. :)

@ptrckvzn
Copy link
Author

I made some change to the webpack files today. Please take a look.

Briefly:

  • not using any environment variables in the npm scripts for better windows compatibility (removed SAGE_ENV)
  • extracted options to a config file
  • dist/ keep the same folder structure (images/, fonts/, styles/, scripts/) - this work better with the assets loaded via php as it expect a subfolder to the dist folder
  • webpack.config has settings specific to the "watch" script
  • renamed the watch script to watch.js

@retlehs
Copy link
Member

retlehs commented Mar 18, 2016

👍 looks good! feel free to PR this repo again from your fork, definitely okay with iterating on here as you're making updates

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.

3 participants