Skip to content

Deploy with Now, inline CSS - #221

Merged
shawnbot merged 77 commits into
release-2.0.1-betafrom
deploy-now
Aug 22, 2018
Merged

Deploy with Now, inline CSS#221
shawnbot merged 77 commits into
release-2.0.1-betafrom
deploy-now

Conversation

@shawnbot

@shawnbot shawnbot commented Aug 21, 2018

Copy link
Copy Markdown
Contributor

This PR builds on #218 with a couple of changes that make it deployable to Now:

  • The build npm script is now called dist (which makes sense, since it builds the dist directory).
  • There is a now.json that configures the project for static deployment out of the docs directory.
  • I've moved all of the Primer CSS "rendering" into our HTML template, eliminating all of the unpkg.com URLs! 🎉

@shawnbot

shawnbot commented Aug 21, 2018

Copy link
Copy Markdown
Contributor Author

FYI, I originally thought that moving all of the Primer CSS styles into our Styles doc component would work, but that breaks some assumptions about stylesheet application order because Emotion puts all of its generated CSS into the <head>.

Apparently—TIL!—styles in <body> get applied after those in <head>, so background-color rules generated by Emotion would be applied before the Primer CSS rules, even when the class names are in the right order (Primer CSS first, then Emotion) and the selectors have the same precedence. 🙃

@shawnbot

Copy link
Copy Markdown
Contributor Author

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. 💥

Before image
After image

@jonrohan

Copy link
Copy Markdown
Member

I was hoping this would mean we wouldn't need to check in the /docs/ folder which would reduce the amount of merge conflicts. Any chance this would work for that?

@shawnbot

Copy link
Copy Markdown
Contributor Author

@jonrohan I think this gets at least one step closer to that goal. I tried getting this to just run x0 in the deployment container, but there were some issues with ports and HTTPS upgrading, so I switched back to a static deployment.

I think Now just runs npm run build to prepare the container, then npm start in deployment. So if we're interested in continuing to build our docs with x0, we could probably make that work by:

  1. Rename build:docs to just build.
  2. Set up npm start to run serve (which Next.js uses for its static deployments) in production.

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?

@shawnbot shawnbot added the ds-reviewed Removes this issue/PR from the first responder list. label Aug 22, 2018
@shawnbot
shawnbot changed the base branch from fewer-scrollbars to release-2.0.1-beta August 22, 2018 20:07
Comment thread .babelrc
"add-react-displayname",
"transform-object-rest-spread"
"transform-object-rest-spread",
"preval"

Copy link
Copy Markdown
Contributor Author

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"

Copy link
Copy Markdown
Contributor Author

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)

Copy link
Copy Markdown
Contributor Author

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.

Comment thread package.json
"lint": "eslint src examples",
"prepublishOnly": "npm run build",
"start": "x0 dev examples -op 8888",
"start": "x0 dev examples -op ${PORT:-8888}",

Copy link
Copy Markdown
Contributor Author

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. ☹️

Comment thread package.json
"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",

Copy link
Copy Markdown
Contributor Author

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.

This was referenced Aug 22, 2018
@shawnbot
shawnbot merged commit ac80e4c into release-2.0.1-beta Aug 22, 2018
@shawnbot shawnbot removed ds-reviewed Removes this issue/PR from the first responder list. status: review needed labels Aug 22, 2018
@shawnbot shawnbot mentioned this pull request Aug 22, 2018
3 tasks

@emplums emplums left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good to me!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants