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 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
  • #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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

akrawchyk commented Mar 20, 2017

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.

Continue suitcss setup and linting config
- 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

This comment has been minimized.

Copy link
Contributor

thomasdegry commented Mar 21, 2017

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

@akrawchyk akrawchyk force-pushed the postcss-webpack branch from 3b61f9d to 2eeb568 Apr 11, 2017

@akrawchyk akrawchyk self-assigned this Apr 11, 2017

@akrawchyk

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

thomasdegry commented Apr 20, 2017

Good ideas! Will get that done tomorrow!

@thomasdegry

This comment has been minimized.

Copy link
Contributor

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!

@akrawchyk akrawchyk referenced this pull request Apr 26, 2017

Closed

Possible nconf bug #62

@thomasdegry thomasdegry added the ready label Apr 26, 2017

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.

This comment has been minimized.

@baconjulie

baconjulie Apr 26, 2017

explicitely --> explicitly

This comment has been minimized.

@thomasdegry

thomasdegry Apr 26, 2017

Contributor

Haha good catch; thank you!

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

This comment has been minimized.

@baconjulie

baconjulie Apr 26, 2017

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

This comment has been minimized.

@akrawchyk

akrawchyk Apr 26, 2017

Contributor

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.

This comment has been minimized.

@baconjulie

```
npm run dev
``` npm run dev

This comment has been minimized.

@marissa-halpert

marissa-halpert Apr 26, 2017

Member

Need a newline between ``` and npm run dev

This comment has been minimized.

@thomasdegry

thomasdegry Apr 26, 2017

Contributor

Good catch! Thanks

}

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

This comment has been minimized.

@marissa-halpert

marissa-halpert Apr 26, 2017

Member

Can we use the hex code instead?

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

This comment has been minimized.

@marissa-halpert

marissa-halpert Apr 26, 2017

Member

Hex code here too!

This comment has been minimized.

@thomasdegry

thomasdegry Apr 26, 2017

Contributor

done in both places

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

This comment has been minimized.

@baconjulie

baconjulie Apr 26, 2017

Is there a reason we are using inline styles here?

This comment has been minimized.

@thomasdegry

thomasdegry Apr 26, 2017

Contributor

@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)

This comment has been minimized.

@marissa-halpert

marissa-halpert Apr 26, 2017

Member

Shhh don't show the secret message!

This comment has been minimized.

@akrawchyk

akrawchyk Apr 26, 2017

Contributor

🙈

--font-size: 1rem;

--primary-color: #43d8f8;
--secondary-color: red;

This comment has been minimized.

@marissa-halpert

marissa-halpert Apr 26, 2017

Member

red here too!

@thomasdegry thomasdegry merged commit 922e180 into master Apr 27, 2017

@thomasdegry thomasdegry deleted the postcss-webpack branch Apr 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment