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

Bye bye Foundation & Browserify, hello Webpack & PostCSS #77

Merged
merged 47 commits into from Apr 27, 2017

Conversation

thomasdegry
Copy link
Contributor

@thomasdegry thomasdegry commented Mar 13, 2017

Idea

There are a couple of things playing here but this PR has some objectives:

  • Kill Foundation and its dependency on old jQuery versions
  • Introduce Webpack 2.x and get rid of Browserify
  • Reduce CSS weight #75

Todo (feel free to add or suggest)

  • Fix linting when running npm run lint
  • Update all the dependencies in package.json and get rid of unused ones >> more or less done; just unsure if we still need/want eslint-config-airbnb, eslint-plugin-react, stylelint-config-standard and stylelint-selector-bem-pattern
  • Figure out whether we need/can differentiate between dev dependencies and normal dependencies
  • Provide generic configuration for use in templates and JS, see SurviveJS webpack environment variables for example
  • Update the README.md file to represent the new boilerplate

Have a small CSS boilerplate that has a normalize and some basic settings WONTFIX, suitcss comes with this by default, see https://github.com/suitcss/suit/blob/master/index.css#L1 -> https://github.com/suitcss/base/blob/master/index.css#L1

@thomasdegry thomasdegry changed the title Our new front end foundation by killing Foundation Bye bye Foundation & Browserify, hello Webpack & PostCSS Mar 13, 2017
@thomasdegry
Copy link
Contributor Author

thomasdegry commented Mar 14, 2017

Interesting fact, because we have a gulpfile.babel.js that is written in EcmaScript it's not easy to update the preset we are currently using from es2015 to es2016 or es2017. Right now we have both es2015 going just to power Gulp and es2017 that is used by webpack to bundle our JS.

Not sure how I feel about having two presets installed but at the other hand it does decouple our build process from the code transpiler so that's good... 🤔

@akrawchyk
Copy link
Contributor

Update all the dependencies in package.json and get rid of unused ones >> more or less done

@thomasdegry let's move the linting modules to devDependencies and add a lint script to npm.

We can potentially remove the stylelint packages since suitcss lints by default... but that happens during the gulp build. Not sure what to do about integrating that into the lint script. We might want to adjust this configuration though, so .stylelintrc will probably need to hang around.

- Removed `.stylelintrc` file and moved everything to a `stylelint.config.js` file so we can consume it for both `npm run lint` as well as our build process
- Added some custom media queries to our `app.css` to make our linter pass without warnings
- Moved all linting stuff to devDependencies and removed some older dependencies for bem support
@thomasdegry
Copy link
Contributor Author

@akrawchyk check out the changes in the last commit; that should take care of a lot of these things!

NODE_ENV: production
dependencies:
override:
- npm install --dev
Copy link
Contributor

Choose a reason for hiding this comment

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

@thomasdegry can we use yarn here instead? https://circleci.com/docs/1.0/yarn/

Also, can you try integrating an example application with circle before we merge this?

@akrawchyk
Copy link
Contributor

akrawchyk commented Apr 20, 2017

@thomasdegry great work, this is looking complete. Along with my comment at #77 (comment) can we also try deploying an example project to Heroku static?

  • Circle
  • Heroku

@thomasdegry
Copy link
Contributor Author

Good ideas! Will get that done tomorrow!

@thomasdegry
Copy link
Contributor Author

thomasdegry commented Apr 21, 2017

@akrawchyk Heroku deploy went smooth. Had to move stylelint and stylelint-config-suitcss to dependencies vs. devDependencies to make it build though.

As for circle, I tried using yarn instead of npm to install the dependencies but ran into this issue with this package failing:

An unexpected error occurred: "https://github.com/pocketjoso/css.git: invalid tar file"

So for now I switched circleci back to npm and the tests linters pass now!

To add more, uncomment the appropriate includes from the `app.scss` file along
with the appropriate settings section for the component in the `_.settings.scss`
file. This is described in detail at [Foundation's Sass docs](http://foundation.zurb.com/sites/docs/sass.html#adjusting-css-output).
As for the JS you need to explicitely indicate which values you want passed to your code via webpack to prevent passing any sensitive data. You can do so by modifying the `webpack.config.js` file, we use the `DefinePlugin` to make our environment variables available in our code. This means you can use `process.env.SECRET_MESSAGE` in your Javascript files and the appropriate environment variable will be substituted during the build process to be shipped in the browser.

Choose a reason for hiding this comment

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

explicitely --> explicitly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha good catch; thank you!

"babelify"{% if cookiecutter.use_foundation_sites == 'y' -%},
"browserify-shim"{%- endif %}
]
"browser-sync": "^2.18.8",

Choose a reason for hiding this comment

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

Should we be using carats in front of the dependencies as it could put us at risk of drifting to different versions/breaking things? https://devcenter.heroku.com/articles/node-best-practices

Copy link
Contributor

Choose a reason for hiding this comment

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

aha! We have a yarn.lock file to avoid different versions being installed on different projects. When you do the initial project cookiecutter everything should start in the same place every time. If the devs choose to upgrade packages after that, they can.

Choose a reason for hiding this comment

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

👍


```
npm run dev
``` npm run dev
Copy link
Contributor

Choose a reason for hiding this comment

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

Need a newline between ``` and npm run dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Thanks

}

@media(--sm-only-viewport) {
color: red;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the hex code instead?

@@ -0,0 +1,3 @@
.Foo {
background-color: red;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hex code here too!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in both places

<div class="Grid Grid--fit">
<div class="Grid-cell">
<header>
<h1 style="margin: 2rem 0 1rem;">The Grid</h1>

Choose a reason for hiding this comment

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

Is there a reason we are using inline styles here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@j-baconator yes; this css needs to live somewhere for documentation purpose so either I "pollute" the main css file; or I create another css file but this html gets deleted on npm build. So this is just to make the grid docs look better

dateDisplayEl.innerHTML = new Date()
document.body.appendChild(dateDisplayEl)

console.log(SECRET_MESSAGE)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shhh don't show the secret message!

Copy link
Contributor

Choose a reason for hiding this comment

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

🙈

--font-size: 1rem;

--primary-color: #43d8f8;
--secondary-color: red;
Copy link
Contributor

Choose a reason for hiding this comment

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

red here too!

@thomasdegry thomasdegry merged commit 922e180 into master Apr 27, 2017
@thomasdegry thomasdegry deleted the postcss-webpack branch April 27, 2017 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants