Closed erikthedeveloper closed 4 years ago
I might have time to provide more details around motivation and overview of before and after behavior, but I'm OOO Friday and Monday so it might not be until Tuesday.
Note: For testing this, I've been copy / pasting the source into zapier/zapier/node_modules/@zapier/babel-preset-zapier/index.js
and running web-ssr simulate:prod
and comparing HTML, CSS output as well as Network tab etc... to prod
Ideally I think we would use yarn link
from z/z
and symlink etc... I'm not sure what we've previously been doing to QA Babel config changes @msholty-fd @vitorbal but that is what I would imagine.
I didn't put too much time into it considering I think we're pretty well sold on absorbing this config into zapier/zapier
I can likely visit that next week (I'm imagining it will be relatively easy)
Ideally I think we would use yarn link from z/z and symlink etc... I'm not sure what we've previously been doing to QA Babel config changes @msholty-fd @vitorbal but that is what I would imagine.
Copy-pasting into node_modules
is what I have been doing too. Linking can be a it finicky, especially when there are monorepos involved.
It's really unfortunate we don't have this package in our monorepo to test this on a review lab before shipping,
If you really want to you can publish a release candidate version and deploy to a reviewlab referencing that, but I agree that it's way more work. Just wanted to point out that it is possible.
Good catch @vitorbal! Tests updated: 0eadb4c
📸 @vitorbal @msholty-fd Comparing generated classnames in prod build (without labelStyle
) with these Babel changes applied. See https://github.com/zapier/zapier/pull/38774
Before https://github.com/zapier/babel-preset-zapier/pull/36:
After https://github.com/zapier/babel-preset-zapier/pull/36:
This gives us consistent, human readable classnames in production without depending on labelStyles, etc...
Changes: