-
Notifications
You must be signed in to change notification settings - Fork 536
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
Deploy with Now, inline CSS #221
Conversation
…to fewer-scrollbars
FYI, I originally thought that moving all of the Primer CSS styles into our Apparently—TIL!—styles in |
Also, I ran a Lighthouse audit in Chrome Devtools on this to compare with primer.github.io/primer-react. The live site scores 61 on performance, but this one scores 80. 💥
|
I was hoping this would mean we wouldn't need to check in the |
@jonrohan I think this gets at least one step closer to that goal. I tried getting this to just run I think Now just runs
I'm totally happy to give this a stab (maybe in a separate PR?), but I'd like to time-box it. @emplums, what do you think? |
@@ -5,6 +5,7 @@ | |||
], | |||
"plugins": [ | |||
"add-react-displayname", | |||
"transform-object-rest-spread" | |||
"transform-object-rest-spread", | |||
"preval" |
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 use this babel plugin to inline the CSS.
) | ||
{examples.map(example => ( | ||
<NavLink | ||
className="menu-item no-underline link-gray-dark" |
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.
Note: only menu-item
comes from primer-navigation. The other classes here come from primer-utilities.
} | ||
|
||
export default Styles | ||
export default withDefaultTheme(Styles) |
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.
Note: This allows the theme to override the base styles, so long as you either wrap <Styles />
in a ThemeProvider or pass theme={theme}
directly.
"build:docs": "x0 build examples --out-dir docs", | ||
"lint": "eslint src examples", | ||
"prepublishOnly": "npm run build", | ||
"start": "x0 dev examples -op 8888", | ||
"start": "x0 dev examples -op ${PORT:-8888}", |
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.
This was an attempt to get the PORT
env variable set by Now working with x0. The problem I ran into once I got that working was that x0's HTTP server doesn't seem to like upgrading from HTTP to HTTPS.
"dist": "NODE_ENV=production rollup -c && npm run build:css", | ||
"predist": "rm -rf dist", | ||
"prepublishOnly": "npm run dist", | ||
"build:css": "primer-module-build --outputDir dist/css src/primer-react.scss", |
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 can use primer-module-build
without any additional configuration to compile our custom CSS bundle. This generates dist/css/build.css
and dist/css/index.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.
Looks good to me!
This PR builds on #218 with a couple of changes that make it deployable to Now:
build
npm script is now calleddist
(which makes sense, since it builds thedist
directory).now.json
that configures the project for static deployment out of thedocs
directory.unpkg.com
URLs! 🎉