-
Notifications
You must be signed in to change notification settings - Fork 27
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
refactor(app-shell): drop Rollup, pre-compile css module and use a macro to load the styles #2018
Conversation
🦋 Changeset detectedLatest commit: 1cf5369 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/commercetools/merchant-center-application-kit/mf6ow6bfq |
compiled-data | ||
dist | ||
dist-tarballs | ||
public | ||
packages/application-shell/test-utils |
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.
The test-utils
is now a preconstruct entrypoint.
getJSON: function (cssFilePath, json) { | ||
const compiledPaths = getCompiledPaths(cssFilePath); | ||
|
||
if (!fs.existsSync(compiledPaths.dir)) { | ||
fs.mkdirSync(compiledPaths.dir); | ||
} | ||
|
||
// This file contains the mapping between the class names referenced | ||
// in the components and the compiled CSS selectors. | ||
fs.writeFileSync( | ||
`${path.join(compiledPaths.dir, compiledPaths.fileName)}.json`, | ||
JSON.stringify(json), | ||
{ encoding: 'utf8' } | ||
); | ||
|
||
// This file provides the type declaration for the JSON file created above. | ||
fs.writeFileSync( | ||
`${path.join(compiledPaths.dir, compiledPaths.fileName)}.json.d.ts`, | ||
`/* eslint-disable prettier/prettier */ | ||
declare const styles: ${JSON.stringify(json)}; | ||
export default styles;`, | ||
{ encoding: 'utf8' } | ||
); | ||
}, |
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.
fs.writeFileSync( | ||
path.join(compiledPaths.dir, compiledPaths.fileName), | ||
compiled.css, | ||
{ encoding: 'utf8' } | ||
); |
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.
Here compiled.css
is the actual compiled CSS code.
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 will still be fingerprinted etc as it's loaded through webpack, right?
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.
No really,this script is what we execute independently.
if (isAuthenticated) | ||
return props.children({ isAuthenticated }); | ||
<> | ||
<GlobalStyles /> |
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.
Injected using Emotion Global
@@ -1,4 +1,3 @@ | |||
import type { SyntheticEvent } from 'react'; |
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.
Preconstruct doesn't like importing React types separately.
const compiledStyles = { | ||
global: fs.readFileSync(path.join(__dirname, './compiled/navbar.css'), { | ||
encoding: 'utf8', | ||
}), | ||
jsonMap: require('./compiled/navbar.css.json'), | ||
}; |
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 is the macro. We're simply returning the loaded CSS and JSON map.
Note that in theory we could have run postcss
here as well (instead of using a custom script). However, the process is async and it would complicate things, as we want here things to be sync.
// https://babeljs.io/blog/2017/09/11/zero-config-with-babel-macros | ||
import compiledStyles from /* preval */ './navbar.styles'; | ||
|
||
const styles = compiledStyles.jsonMap; |
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.
In the code, we reference the class names using the JSON map.
Bonus now is that we also get autocompletion and type checks.
<Global | ||
styles={css` | ||
${compiledStyles.global} | ||
`} | ||
/> |
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.
Since we pre-compiled the CSS styles, we need to inject them globally here.
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.
Thanks. I can follow along from a setup perspective. Macros are somewhat magical to me still :)
"build:bundles:watch": "yarn build:bundles -w", | ||
"build:test-utils": "cross-env NODE_ENV=development rollup -c ../../rollup.config.js -i ./src/test-utils/index.ts", | ||
"build:typings": "cross-env tsc -p tsconfig.declarations.json --emitDeclarationOnly --declarationDir dist/typings", | ||
"postbuild:typings": "echo \"export * from '../dist/typings/test-utils';\" > test-utils/index.d.ts" |
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.
💯
fs.writeFileSync( | ||
path.join(compiledPaths.dir, compiledPaths.fileName), | ||
compiled.css, | ||
{ encoding: 'utf8' } | ||
); |
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 will still be fingerprinted etc as it's loaded through webpack, right?
I don't even know what a macro is (except via Sass or Less) 😄 |
This article was quite nice: https://kentcdodds.com/blog/write-your-own-code-transform |
It took a couple of more rounds of reading, and something I would have liked to see at the PR. but here is what I understood
questions
side note
|
See #2018 (comment)
What what? PS: note that this is a "temporary" solution, as we want to use Emotion eventually and no CSS modules. See #1884
Yes
Yes, because it's the only component that is always rendered (see our internal auth app, which does not use AppShell). |
af72573
to
dd183a5
Compare
aah... nice that is actually elegant. PS. can't say it's genius, however 🤷🏽♂️ #slap |
…cro to load the styles
…om postcss-loader
dd183a5
to
1cf5369
Compare
So I finally figured out a way to bundle the app-shell package using preconstruct.
Until now it was not possible because we loaded CSS modules, and thus babel does not know how to process that import.
Now we don't directly load the
.css
file anymore. Instead, we usepostcss
to compile it and load the styles using a macro. This allows the code to be bundled using Babel.