Warn when using overlapping styles (e.g. border and borderBottom) #6348

gaearon · 2016-03-26T18:25:22Z

This "good first bug" is taken by @YongPilMoon. Don't work on it unless that's you!

This has been discussed a few times before but I don’t think there was any conclusion, and PRs intending to solve the issue were not merged for various reasons.

I would like to close those PRs as outdated, and reboot the discussion about this.
Performance considered, the conclusion from @sebmarkbage and @syranide seems to be:

We should consider not allowing conflicting style rules at all.

— #2013 (comment)

IMHO, all things considered it's better to just disallow overlapping and warn in dev.

— #4661 (comment)

Radium by @ianobermiller came to the same conclusion in FormidableLabs/radium#95 but there was some backlash afterwards. React Native seems to allow style expansions but only for a few attributes (e.g. margin and padding, but not border).

I’m closing old pull requests about this, and creating this issue to track implementation of the behavior we seem to agree upon: we should warn in __DEV__ when border and borderBottom are used at the same time. We can discuss more specifics (should either be ignored? should we allow a few whitelisted properties but warn for others?) in this issue.

As a migration strategy, we can suggest people to use something like https://github.com/ActionIQ/style-builder if they really need those shortcuts. It’s also something we’ll need to decide upon when implementing integrated styling.

Related issues:

Related PRs:

The text was updated successfully, but these errors were encountered:

mattborn · 2016-03-28T21:27:13Z

@gaearon Thank you for resurfacing this! Crazy that it has been a year since I opened the Radium issue.

I like the idea of warning about shorthand properties like border, but not borderBottom. There shouldn’t be any surprises when merging style objects using the longer property names.

Question. Is the intent of this issue to address only the analysis of individual style objects or is there also a desire to analyze and warn about possible collisions of properties that are conditionally applied?

gaearon · 2016-03-28T22:43:04Z

Question. Is the intent of this issue to address only the analysis of individual style objects or is there also a desire to analyze and warn about possible collisions of properties that are conditionally applied?

Not sure I understand the question, can you elaborate?

mattborn · 2016-03-29T00:40:22Z

Sure. If it is invalid to have both border and borderColor on the same element, then it makes sense to warn about this. In my experience, this combination is easy to avoid, but it is more difficult to anticipate and debug when styles are being applied dynamically (conditional logic based on props or state, media queries, Radium, etc.) or passed down as props and merged with lower order style objects. If a warning for the shorthand border attribute occurs when there are no other attributes that might affect the border involved, then the warning seems more like a dogmatic rule than a helpful debugging tool since shorthand border is still technically valid. However, if there is an attribute in a separate style object—like changing borderColor on hover for example—that would affect the border involved and will get applied at runtime to the node that already has shorthand border on it, I’d like to be warned as early as possible as this is where things tend to break for me.

gaearon · 2016-03-29T01:29:27Z

Ah, good point. I’d probably warn only if we know the styles overlap but I’m not sure how easy it is to tell. I haven’t thought about it much and I don’t have a good solution.

jimfb · 2016-03-29T10:33:13Z

@gaearon An alternate solution that I like better is to use csstext as per #5397, which IIRC cleanly solves a superset of these issues.

aweary · 2016-03-29T16:48:49Z

What about situations where a third-party component defines styles and allows overrides? Say a component defines its own border style and I want to just override the borderBottom style?

rtsao · 2016-09-13T23:04:37Z

I've started working on dead-simple helper functions to convert shorthand properties to longform (among other things): https://github.com/rtsao/lostyle/tree/master/src (docs)

- Prevent using style conflicting properties like border and border Button facebook/react#6348 - Avoid using ES6 features that can't be transpiled like includes - Remove dead style properties

sophiebits · 2018-05-24T01:55:15Z

This is super unpredictable and confusing and I just ran into this.

I'd suggest we at least warn on updates when we know it's problematic? That is, if we go from {background: a, backgroundSize: b} to {background: c, backgroundSize: b}, warn that backgroundSize is being overridden (but if backgroundSize changes at the same time, don't warn). We can do this for all property shorthands and know whether it's problematic from the property enumeration order.

sophiebits · 2018-05-24T18:25:11Z

Marking as "good first issue". Let's add a DEV-only warning for this per my comment above. I think a good place to begin investigating would be here:

https://github.com/facebook/react/blob/v16.4.0/packages/react-dom/src/client/ReactDOMFiberComponent.js#L619

If you'd like to take this on, please comment here so that we don't have two people working on this.

supertinou · 2018-05-24T18:45:19Z

@sophiebits, I am happy to work on this one :)

sophiebits · 2018-05-24T18:46:33Z

@supertinou It's yours!

craigkovatch · 2018-06-13T02:17:03Z

Just hit this and was indeed surprised and confused by it.

sophiebits · 2018-06-13T05:22:34Z

@supertinou Just wanted to check if there's anything I can help you with on this.

mirkojotic · 2018-06-18T17:23:52Z

@sophiebits Hi I don't want to step on anybodies toes :D but I've taken a look into this issue, replicated it and would love to take a stab at it but I don't know if @supertinou is still working on it.

giuseppeg · 2018-08-03T07:41:12Z

I made a codepen to recap/explain what is going on https://codepen.io/giuseppegurgone/pen/mjKbmq?editors=0010 @gaearon / @sophiebits let me know if I forgot something.

sophiebits · 2018-09-27T22:29:06Z

@mirkojotic Still interested?

logic0319 · 2018-09-28T07:02:24Z

If there is no one i would like to take this one :)

sophiebits · 2018-10-09T02:14:45Z

All right @YongPilMoon, it's yours.

sophiebits · 2018-10-09T02:16:55Z

Adding a note here: removing a property can also trigger this bug. https://stackoverflow.com/q/52636618/49485

This is one of the most insidious quirks of React DOM that people run into. Now we warn when we think an update is dangerous. We still allow rendering `{background, backgroundSize}` with unchanging values, for example. But once you remove either one or change `background` (without changing `backgroundSize` at the same time), that's bad news. So we warn. Fixes facebook#6348.

This is one of the most insidious quirks of React DOM that people run into. Now we warn when we think an update is dangerous. We still allow rendering `{background, backgroundSize}` with unchanging values, for example. But once you remove either one or change `background` (without changing `backgroundSize` at the same time), that's bad news. So we warn. Fixes #6348.

This is one of the most insidious quirks of React DOM that people run into. Now we warn when we think an update is dangerous. We still allow rendering `{background, backgroundSize}` with unchanging values, for example. But once you remove either one or change `background` (without changing `backgroundSize` at the same time), that's bad news. So we warn. Fixes facebook#6348.

gaearon · 2020-02-26T21:03:27Z

The warning is out in 16.13.
https://reactjs.org/blog/2020/03/02/react-v16.13.0.html

- Prevent using style conflicting properties like border and border Button facebook/react#6348 - Avoid using ES6 features that can't be transpiled like includes - Remove dead style properties

gaearon added Type: Bug Component: DOM labels Mar 26, 2016

gaearon mentioned this issue Mar 26, 2016

Restores 'border' when 'borderColor' is removed #2013

Closed

gaearon added the HTML label Mar 26, 2016

This was referenced Mar 26, 2016

fixed the issue that inline-styles updating could lose style properties #4661

Closed

Decompose shorthand/compound styles to their constituent parts FormidableLabs/radium#95

Open

oliviertassinari mentioned this issue Apr 15, 2016

React 15+ inline styling is producing concatenated style properties from separate, unique object keys #6524

Closed

pwlmaciejewski mentioned this issue Oct 19, 2016

Shorthand props to longform question catamphetamine/react-styling#3

Closed

linmic mentioned this issue Nov 21, 2016

Fail to update DOM style attribute #8362

Closed

oliviertassinari mentioned this issue Nov 30, 2016

[core] Apply 3 different fixes mui/material-ui#5695

Merged

3 tasks

bvaughn mentioned this issue Jan 5, 2017

Shorthand CSS properties can cause style inconsistencies #8689

Closed

syranide mentioned this issue Feb 8, 2017

Inline border style not rendering correctly #8957

Closed

aweary mentioned this issue Mar 30, 2017

Bug: Updating background style ignores backgroundSize #9295

Closed

aweary mentioned this issue Jul 10, 2017

animationDelay not applying #10134

Closed

sophiebits added the good first issue label May 24, 2018

sophiebits added good first issue (taken) and removed good first issue labels May 24, 2018

sophiebits added good first issue and removed good first issue (taken) labels Sep 27, 2018

sophiebits added good first issue (taken) and removed good first issue labels Oct 9, 2018

logic0319 added a commit to logic0319/react that referenced this issue Oct 16, 2018

Add warning about css shorthand properties collision(facebook#6348)

4fe3708

logic0319 mentioned this issue Oct 19, 2018

Add warning about css shorthand properties collision(#6348) #13860

Closed

sophiebits mentioned this issue Nov 9, 2018

Warn about conflicting style values during updates #14181

Merged

sophiebits closed this as completed in #14181 Nov 9, 2018

fdietze mentioned this issue Aug 7, 2019

Margin shorthands are not patched correctly snabbdom/snabbdom#441

Closed

rdsedmundo mentioned this issue Aug 14, 2019

Shorthand CSS property collision should trigger a warning #16393

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Warn when using overlapping styles (e.g. border and borderBottom) #6348

Warn when using overlapping styles (e.g. border and borderBottom) #6348

gaearon commented Mar 26, 2016 •

edited by sophiebits

mattborn commented Mar 28, 2016

gaearon commented Mar 28, 2016

mattborn commented Mar 29, 2016

gaearon commented Mar 29, 2016

jimfb commented Mar 29, 2016

aweary commented Mar 29, 2016

rtsao commented Sep 13, 2016 •

edited

sophiebits commented May 24, 2018 •

edited

sophiebits commented May 24, 2018

supertinou commented May 24, 2018

sophiebits commented May 24, 2018

craigkovatch commented Jun 13, 2018

sophiebits commented Jun 13, 2018

mirkojotic commented Jun 18, 2018

giuseppeg commented Aug 3, 2018 •

edited

sophiebits commented Sep 27, 2018

logic0319 commented Sep 28, 2018 •

edited

sophiebits commented Oct 9, 2018

sophiebits commented Oct 9, 2018

gaearon commented Feb 26, 2020

Warn when using overlapping styles (e.g. border and borderBottom) #6348

Warn when using overlapping styles (e.g. border and borderBottom) #6348

Comments

gaearon commented Mar 26, 2016 • edited by sophiebits

This "good first bug" is taken by @YongPilMoon. Don't work on it unless that's you!

mattborn commented Mar 28, 2016

gaearon commented Mar 28, 2016

mattborn commented Mar 29, 2016

gaearon commented Mar 29, 2016

jimfb commented Mar 29, 2016

aweary commented Mar 29, 2016

rtsao commented Sep 13, 2016 • edited

sophiebits commented May 24, 2018 • edited

sophiebits commented May 24, 2018

supertinou commented May 24, 2018

sophiebits commented May 24, 2018

craigkovatch commented Jun 13, 2018

sophiebits commented Jun 13, 2018

mirkojotic commented Jun 18, 2018

giuseppeg commented Aug 3, 2018 • edited

sophiebits commented Sep 27, 2018

logic0319 commented Sep 28, 2018 • edited

sophiebits commented Oct 9, 2018

sophiebits commented Oct 9, 2018

gaearon commented Feb 26, 2020

gaearon commented Mar 26, 2016 •

edited by sophiebits

rtsao commented Sep 13, 2016 •

edited

sophiebits commented May 24, 2018 •

edited

giuseppeg commented Aug 3, 2018 •

edited

logic0319 commented Sep 28, 2018 •

edited