-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Update css-to-react-native to v3.0.0 #2811
Update css-to-react-native to v3.0.0 #2811
Conversation
@omBratteng can you change the target branch to v5 (canary) and copy over the notes from the OP into a changelog entry? |
@probablyup I've rebased my branch to be canary and recommited the dependency update. If @jacobp100 could provide a changelog I'd add it in my PR |
Hey, the changelog is here |
@omBratteng just need that changelog entry with the copied notes and we're good to ship this |
@probablyup do you mean to add the changelog from css-to-react-native to the |
Summarize it yes
…On Tue, Oct 29, 2019 at 4:43 AM Ole-Martin Bratteng < ***@***.***> wrote:
@probablyup <https://github.com/probablyup> do you mean to add the
changelog from css-to-react-native to the CHANGELOG.md file?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#2811?email_source=notifications&email_token=AAELFVVLGOFPBLNPIMMGOBLQQ7ZUBA5CNFSM4JBIR6S2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECPV2BY#issuecomment-547314951>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAELFVXCWSJUFJBXVB4RPV3QQ7ZUBANCNFSM4JBIR6SQ>
.
|
Thanks! |
Thanks! 😄 |
@omBratteng @probablyup @jacobp100 it's more than unitless line heights that this removed support for. this removed support for all unitless properties. this is a major breaking change in between two minor releases and it makes things difficult because there were important patches that i can't get now between the newer versions and the one that made this change. is it possible to save this for the production release? it is going to require a significant refactor and possibly a codemod. |
That is why it is being shipped on the v5 major branch not v4. You can always pin back to the prior beta until you get a chance to make relevant updates. @jacobp100 I'm not a fan of the unitless thing being a unilateral change though... your changelog made it sound like it would only affect the font shorthand. |
I thought that was clear We want to get closer to web to avoid these huge changes in the future. For example, if we ever enable the border radius transform on styled-components, that above example would suddenly break with a red-screen error |
@jacobp100 Thanks for posting about that codemod, very helpful! I also added it to the github release notes for rc0 |
Guys react native still requires unitless styles. So you’re saying our RN components must be unitless and our styled components must have units. I don’t think it makes sense. The two will be confusing together but ultimately it should be a personal decision.
Codemod breaks for typescript (interface reserved word)
… On Dec 3, 2019, at 11:03 AM, Evan Jacobs ***@***.***> wrote:
@jacobp100 Thanks for posting about that codemod, very helpful! I also added it to the github release notes for rc0
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@shamilovtim There’s a lot of discussion and reasoning over why we require units over on the css-to-react-native repo. It’s worth a read The codemod uses jscodeshift (the first step is you installing this). There does seem to be a good amount on google about getting it to run on TS files. If you want to try it out, let me know how it goes! It would be great to add docs for other people doing the same 🙂 |
I created a patch for now css-to-react-native+3.0.0.patch: diff --git a/node_modules/css-to-react-native/index.js b/node_modules/css-to-react-native/index.js
index 0d6f377..cea064e 100644
--- a/node_modules/css-to-react-native/index.js
+++ b/node_modules/css-to-react-native/index.js
@@ -792,7 +792,6 @@ var transformRawValue = function transformRawValue(propName, value) {
if (needsUnit && isNumberWithoutUnit) {
// eslint-disable-next-line no-console
- console.warn("Expected style \"" + propName + ": " + value + "\" to contain units");
}
if (!needsUnit && value !== '0' && !isNumberWithoutUnit) {
diff --git a/node_modules/css-to-react-native/src/index.js b/node_modules/css-to-react-native/src/index.js
index 0afce0a..351f45d 100644
--- a/node_modules/css-to-react-native/src/index.js
+++ b/node_modules/css-to-react-native/src/index.js
@@ -19,7 +19,6 @@ export const transformRawValue = (propName, value) => {
const isNumberWithoutUnit = numberOnlyRe.test(value)
if (needsUnit && isNumberWithoutUnit) {
// eslint-disable-next-line no-console
- console.warn(`Expected style "${propName}: ${value}" to contain units`)
}
if (!needsUnit && value !== '0' && !isNumberWithoutUnit) {
// eslint-disable-next-line no-console I'll try to get the codemod working once I have a clean git tree and I'll update the docs once I get it to work |
You can just use https://facebook.github.io/react-native/docs/debugging#warnings |
No dice it spams and clogs my console with dozens of errors which seem to be blocking. Makes watchman chug
… On Dec 3, 2019, at 3:18 PM, Jacob Parker ***@***.***> wrote:
You can just use https://facebook.github.io/react-native/docs/debugging#warnings
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
This should close #2782