Initial work toward new component bundle directories #2859
Conversation
|
@fzzzy - Still got some work to do here, but I think this is doing the right things so far with styles, stories, & tests. |
|
What on earth is going on in this failure here? o.O |
| rules: [ | ||
| { | ||
| test: /\.(css|scss)$/, | ||
| loaders: ["style-loader", "css-loader", "sass-loader"], |
There was a problem hiding this comment.
The sass-loader docs gave something very different than this. I never would have figured this out...
There was a problem hiding this comment.
Ignore that... it didn't work and shouldn't have been there - I was editing the wrong file in the wrong dir & accidentally committed it
There was a problem hiding this comment.
FWIW, these are the changes to the main webpack config, basically from the sass-loader docs: https://github.com/mozilla/testpilot/pull/2859/files#diff-11e9f7f953edc64ba14b0cc350ae7b9d
There was a problem hiding this comment.
Yeah that changeset looks really good. I think I can actually understand how it works!
lmorchard
force-pushed
the
2807-experimentdetails-breakdown
branch
2 times, most recently
from
5b9af0f
to
9f3beda
Compare
Looks like the test modules are getting caught up in the |
|
Looking at the diff, I'm guessing the hacks you have in test-setup were what I needed to fix the weird problem I had trying to load sass earlier today. I like the look of the way webpack is set up here better than what I had, too. |
- Storybook now also finds stories in frontend/src/app/**/stories.js - `npm run test` now also finds tests in frontend/src/app/**/tests.js - New /static/app/app.js.css stylesheet built from Sass styles imported by components - Hacks to ignore .scss imports when using component modules outside Webpack for static page build & tests - Update flowconfig to ignore .scss imports - Small eslint upgrade so CLI matches gulp-eslint Issue mozilla#2807
lmorchard
force-pushed
the
2807-experimentdetails-breakdown
branch
from
9f3beda
to
bd61781
Compare
|
Okay. Reorg'd this into a few more self-contained commits and cleaned some things up. And yay the tests pass!
Looks like we have #2808 for porting over SCSS - I'll say this PR enables index.scss per component directory, but we still have to rework the existing SCSS a bit to use the capability. |
lmorchard
changed the title
lmorchard
force-pushed
the
2807-experimentdetails-breakdown
branch
from
bd61781
to
ceaff1b
Compare
ghost
added this to the
lmorchard
force-pushed
the
2807-experimentdetails-breakdown
branch
from
8343e0c
to
f8c29c9
Compare
|
I think this is ready to land. I would be fine with just linking to the gist in the documentation for now. |
|
This could use some revision, but I just copypasta'd most of that proposal into the Storybook docs. Might just call that good enough and merge after tests go green |
|
Seems good enough for now! |
Some remaining TODOs:
Actually migrate over some styles for the new bundles componentsDone so far:
Moved ExperimentRowCard and UpdateList components into subdirectories
Extracted IncompatibleAddons as a component from ExperimentPage
Webpack config to extract static/app/app.js.css from component imports
Link to static/app/app.js.css styles from the static page template
Storybook config changes to use frontend/src/app/**/{ComponentName}/stories.jsx files
Mocha test command changes use frontend/src/app/**/{ComponentName}/tests.js
Hacks to frontend/tasks/pages.js to ignore scss imports
Hacks to test-setup.js to ignore scss imports
See also:
https://gist.github.com/chuckharmston/01e5b7a16ba9b771863cbb8b2de64138
Issue #2807