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
Consume changes from terra-toolkit and terra-dev-sever #156
Conversation
package.json
Outdated
@@ -44,11 +44,11 @@ | |||
"clean:obsolete-snapshots": "npm test -- -u", | |||
"compile": "npm run compile:src", | |||
"compile:src": "lerna run compile", | |||
"compile:prod": "NODE_ENV=production node --max_old_space_size=2500 node_modules/webpack/bin/webpack.js --config webpack.prod.config.js -p", | |||
"compile:prod": "NODE_ENV=production node --max_old_space_size=3000 node_modules/webpack/bin/webpack.js --config node_modules/terra-dev-site/src/webpack/webpack.config.js -p", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyone have a good idea for aliasing out this file to something more friendly without having to include a webpack config locally?
node_modules/terra-dev-site/src/webpack/webpack.config.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe provide a terra-dev-site script that will wrap webpack such that it takes args ?.
Thinking
compile:prod: 'NODE_ENV=production node --max_old_space_size=3000 tds-prode'
which maps to
tds-prod.js which calls
shelljs(webpack.js --config ../../src/webpack/webpack.config.js -p)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually should we be using the servestatic script here by chance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought about wrapping, but I want to avoid wrapping webpack because i don't want ot duplicate all the different webpack config options that exist (there are a ton).
Serve static could be used, but creating the artifacts is a by-product of running the server and there are cases where we just want the build output (deploying to gh-pages).
…ersions of components? thus my update of props table.
package.json
Outdated
@@ -38,17 +38,17 @@ | |||
} | |||
}, | |||
"scripts": { | |||
"bootstrap": "lerna bootstrap", | |||
"bootstrap": "lerna bootstrap --hoist", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd hold of on using the --hoist flag until we can verify it works with the theme repos. Right now, I know it works with terra-ui, but I wasn't able to get it working the cerner-consumer-theme repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, but we should pause this PR until we can verify --hoist
works. It significantly lowers the memory footprint of the dev site.
@@ -46,6 +46,6 @@ | |||
"lint:scss": "stylelint src/**/*.scss", | |||
"test": "npm run test:jest && npm run test:wdio", | |||
"test:jest": "jest ./tests/jest/* --config ../../jestconfig.json", | |||
"test:wdio": "wdio ../../wdio.conf.js" | |||
"test:wdio": "../../node_modules/.bin/wdio ../../node_modules/terra-dev-site/config/wdio/wdio.conf.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make this change in the generator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, but the downside is that it's different between core and framework because core uses it's own wdio config and framework uses the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😞 yeah...
Summary
This PR pull's in the new terra-toolkit changes and terra-dev-site changes.
More deletions than additions. :)
Thanks for contributing to Terra.
@cerner/terra