Skip to content
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

Transient props #2093

Closed
wants to merge 5 commits into from

Conversation

@probablyup
Copy link
Contributor

probablyup commented Oct 12, 2018

This commit introduces a new (currently styled-components-specific) concept called "transient props". When a prop is prefixed with a dollar sign (e.g. $foo={false}), it will only be applied to the immediate component and not forwarded to any deeper children or layers of the component stack.

In the case of styled-components, this equates to a styling-only prop that you may consume in your style interpolations but will never end up on the DOM node or in your wrapped component (other props can still be accessed normally). For example:

const Input = styled.input`
  color: ${props => props.$color};
  background: ${props => props.disabled ? '#DDD' : '#FFF'};
`

<Input $color="#333" /> // => <input />, $color not passed through
<Input $color="#333" disabled /> // => <input disabled />, disabled passed through

This would allow us to get rid of the prop-whitelist in the next major version (v5?), which would considerably drop the bundle size of the package.

Fixes #2131
Fixes #2129
Fixes #439

This commit introduces a new JSX concept called "transient props". When
a prop is prefixed with a $ (e.g. $foo), it will only be applied to
the immediate component and not forwarded to any deeper children or
layers of the component stack.

In the case of styled-components, this equates to a styling-only prop
that you may consume in your style interpolations but will never end
up on the DOM node or in your wrapped component. Other props can still
be accessed normally.
@probablyup

This comment has been minimized.

Copy link
Contributor Author

probablyup commented Oct 12, 2018

This could also potentially land in v4.x but without the existing prop validation being removed. If it catches on we can deprecate prop validation and remove it in v5.

@cloud-walker

This comment has been minimized.

Copy link

cloud-walker commented Oct 12, 2018

It is a JSX concept coming from react? Or just a styled-components JSX concept?

@Jessidhia

This comment has been minimized.

Copy link
Member

Jessidhia commented Oct 12, 2018

Just a styled-component's thing; the only props that have any special meaning in JSX are key and ref.

Copy link
Member

mxstbr left a comment

This is an interesting concept, kinda our namespacing idea but it works and doesn't require JSX v2, interesting interesting. Is there precedent for this, is anybody else doing it? Who else could/should be doing it?

/cc @gaearon

@chengjianhua

This comment has been minimized.

Copy link

chengjianhua commented Oct 12, 2018

Can I make a request? Why not _ or __? The style of using sign $ feel likes not be consistent with preferences of most of us (use camelCase almost, a little underscore_case...) 🙈. Let me remember jQuery or Angular or Vue...

@mxstbr mxstbr changed the title [v5 candidate] transient props [RFC] Transient props with prefixes Oct 12, 2018
@mxstbr mxstbr changed the title [RFC] Transient props with prefixes [RFC] Transient props Oct 12, 2018
@diondiondion

This comment has been minimized.

Copy link

diondiondion commented Oct 12, 2018

Is the as prop going to become $as?

And is there still a chance you'll also implement a static "prop-filtering at component definition time" solution similar to what's described here, for people like me who really don't want to expose styled-component-specific props in their UI components?

(For example like this:

styled(Comp).filterProps(['customProp1', 'customProp2'])`
  /* styles */
`;

)

On a side-note, the as prop could also do with a way of being forwarded to the underlying component instead of applying directly to the one it's attached to.

@probablyup

This comment has been minimized.

Copy link
Contributor Author

probablyup commented Oct 12, 2018

@kitten

This comment has been minimized.

Copy link
Member

kitten commented Oct 12, 2018

💯 Still a big fan of this ❤️

@probablyup

This comment has been minimized.

Copy link
Contributor Author

probablyup commented Oct 12, 2018

Is the as prop going to become $as?

We can definitely look into that. That particular change would have to be in v5 if we do land it.

@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Oct 12, 2018

Just realised a downside to this approach: it makes the abstraction leaky by making styled-components very different from normal React component. That in turn makes it hard to evolve that component without breaking changes to any user of the component.

To elaborate, imagine a Button component with a primary property. At first, your implementation would probably be something like this:

export const Button = styled.button``;

So throughout the whole application you use the Button like so:

<Button $primary disabled />

Now imagine you get some new requirements and your Button needs some custom behaviour, so you change the implementation to something like this:

const StyledButton = styled.button``;

export const Button = (props) => (
  <StyledButton>{/* ... */}</StyledButton>
)

Now you're stuck in a bad place since you have only got these two options:

  1. Continue naming the prop $primary, pretending as though the component was a styled one. Unfortunately, it's not, so people who think it is will have a bad time, e.g. when trying to use it in a component selector ${Button}
  2. Rename the prop to primary and have to comb through the entire application and however many hundreds of usages to change the prop name

That sucks. 😕


I'm leaning towards the "style-prop" approach (a single blacklisted prop that's always filtered and everything else is passed through), specifically a prop that doesn't have any s-c specific connotation (I like variant). It keeps the abstraction intact and makes for much easier evolution of existing components.

@probablyup

This comment has been minimized.

Copy link
Contributor Author

probablyup commented Oct 12, 2018

by making styled-components very different from normal React component

It's a choice to be made by the developer. If they need multiple layers of components to receive the prop, they should not mark it as transient and just filter it out at the appropriate level so it doesn't hit the DOM.

@diondiondion

This comment has been minimized.

Copy link

diondiondion commented Oct 12, 2018

Hint: Maybe this whole dance around custom props wouldn't be a problem if you

  • provided a nice native way to filter props in the styled-component definition
  • educated people about why this is necessary

You're probably as tired of hearing me repeat this as I am repeating it, but I'll do it once more:

To me it is SO obvious that both custom prop and prefixed prop solutions are hacky workarounds around the fact that developers HAVE to manage (i.e. decide whether to pass on or not) their component's props at some point. I much prefer doing this once when I write the component over doing/thinking about it EVERY SINGLE TIME I use the component: "Wait, is this component a styled component? Is this prop an s-c-exclusive styling prop? Guess I have to go and look it up..." What @mxstbr says above hits the nail on the head even though I don't agree with his conclusion because nesting a prop especially for s-c isn't all that different from giving it a prefix.

React devs are used to some boilerplate (constructor(props) {super(props); ...}), listing a component's transient props isn't going to kill us, and it's going to make this unavoidable process super explicit.

🎤

@styled-components styled-components deleted a comment from probablyup Oct 12, 2018
@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Oct 15, 2018

It's a choice to be made by the developer. If they need multiple layers of components to receive the prop, they should not mark it as transient and just filter it out at the appropriate level so it doesn't hit the DOM.

A big part of what makes styled-components so nice in my mind is that it's "Just React Components™️". I would not want to require users to choose upfront whether they'll ever have more complex requirements—those are always changing, so I think they'd end up always having to wrap each styled component they export, which is not great.

nesting a prop especially for s-c isn't all that different from giving it a prefix.

That would be the case if the prop was called "sc" or something like that, but not if giving it a generic name like variant="primary". By doing that you don't end up in the bad place I described above, as whether or not the component is a styled component or not is an implementation detail. (every component can have a variant prop) In my mind that means it solves the problem?

@diondiondion

This comment has been minimized.

Copy link

diondiondion commented Oct 15, 2018

That would be the case if the prop was called "sc" or something like that, but not if giving it a generic name like variant="primary".

Sure, every component can have a variant prop, but the truth is that normally every component wouldn't have one – unless you made it a convention across your codebase to move all custom component props into a variant prop. Since that's not very likely, the variant prop would still feel arbitrary and come with the same confusing downsides as a prefix, just in a more neutral guise. In practice I'd still have to know or think about whether a prop for a given component is attached directly or as part of the variant object.

So I'd still be one of those who'd

end up always having to wrap each styled component they export, which is not great.

@flybayer

This comment has been minimized.

Copy link

flybayer commented Oct 15, 2018

I agree with @diondiondion. The default, standard way of passing props to a component is passing them directly, not as a nested object like variant. While the variant solution is definitely a workable solution, I don't think it's ideal.

I'd much prefer to leave props as is, and, instead, add a native SC way for users to blacklist props in the component definition that shouldn't end up in the dom.

Imo, the problems with variant are:

  1. You have to think about how to access a certain prop (like if I need to wrap a component for something). Is the prop I need at the root level or nested in variant?
  2. It'll be very confusing when someone invariably adds the same prop name at the root level and in variant.
@probablyup probablyup referenced this pull request Oct 15, 2018
@probablyup probablyup changed the base branch from develop to master Oct 15, 2018
@probablyup probablyup referenced this pull request Oct 16, 2018
8 of 14 tasks complete
bpierre added a commit to aragon/aragon that referenced this pull request Oct 19, 2018
Easiest solution for now, until we get transient props [1].

[1] styled-components/styled-components#2093
bpierre added a commit to aragon/aragon that referenced this pull request Oct 19, 2018
…#404)

Easiest solution for now, until we get transient props [1].

[1] styled-components/styled-components#2093
@probablyup probablyup removed the 5.0 label Oct 21, 2018
@probablyup

This comment has been minimized.

Copy link
Contributor Author

probablyup commented Nov 13, 2018

Unfortunately this doesn't solve the central issue of multiple levels of components consuming the same sets of props... it only supports 2 layers, where in practice there can be more than that. Going to close for the time being and seek further alternatives. For the record, no other proposal has addressed the 3+ layer issue either, other than JSX namespacing which I think the most correct solution but requires toolchain support from Babel, etc.

@probablyup probablyup closed this Nov 13, 2018
@geelen

This comment has been minimized.

Copy link
Member

geelen commented Nov 20, 2018

Hey so, I've been pretty quiet on this for a while, mainly because I really want JSX namespace props and thought that if I looked away for long enough they'd be implemented by the time I look again, alas!

I think putting this in terms of "transient" props is actually a really good one–all we're then doing is defining a convention for a prop that can only be passed a single level deep. Using the $ character is really the only option given that # is a syntax error. But this has a couple of nice properties:

  • It's extremely close to the existing implementation.
    This is important because as far as I can see it, dropping the whitelist is going to cause a boatload of runtime warnings, with no good codemod possible. So the migration path is going to be awkward for everyone, let's make it as less awkward as possible.
  • It's dumb as a plank.
    "Props starting with $ don't get passed through. All other ones do". As long as all props are available inside interpolations, it's an easy decision to make.
  • It can be customisable.
    @diondiondion is right that "developers HAVE to manage (i.e. decide whether to pass on or not) their component's props at some point", but for the base case where SC is wrapping a div or button , adding that info on every definition just makes things clumsy. When you're building up more complex component APIs on the other hand, you might wanna change things:
styled.button` ... ` // equivalent to:
styled.button.rejectProps(
  propName => propName .startsWith('$')
)` ... ` // default

// works great for <button large type="submit">

styled(Comp).rejectProps(
  propName => new Set(['dont','pass','these']).has(propName)
)` ... ` // overridden

// works better for <Component dont pass these/>

Plus we could also do some things like allowing anything specified in attrs to be added after this has been run, so you could do:

styled.div.attrs({
  $someProp: props => props.$someProp
})` ...` 

This would be pretty easily wrapped up in a helper:

styled.div.attrs({
  ...passProp('$someProp', '$someOtherProp')
})` ...` 

Now for some more controversial ideas:

  • We should alias props.x to match the prop named $x
    In other words, ${props => props.foo} matches both <Comp foo/> and <Comp $foo/>. This would dramatically simplify the migration path since its only callsites that need updating, not components. This better fits the concept of a "transient prop" rather than an actual SC-specific API. It just means "don't pass this on" rather than actually being a new name.
  • Alternatively, let's do more babel
    We could technically add JSX namespaces to our babel plugin, I believe. Maybe $propName is the no-babel version and style:prop or some other syntax is the "preferred" one. I hate when other people introduce non-standard syntax to something they don't own so yeah this is probably a bad idea.
  • Maybe $ is our new thing?
    Last year I played around with a much more Sass-y version of SC syntax but I couldn't figure out the parser grammar required to actually implement it:
styled.div`
  color: $color || black;
  $bold? {
    font-weight: 700;
  }
  opacity: $variant = 'subtle' ? 0.8 : 1.0;
`

// compiles to
styled.div`
  color: ${ props => props.$color || props.theme.color || 'black' };
  ${ props => props.$bold || props.theme.bold && css`
    font-weight: 700;
  ` }
  opacity: ${ props => (props.$variant || props.theme.variant) === 'subtle' ? 0.8 : 1.0 };
`

Bit wild I know, and totally just an initial thought about a syntax, but $ would work really well there (since it's the start of the interpolation syntax anyway), so maybe using $ just for transient props isn't the worst thing to be starting with...

Anyway, just a few thoughts. 👋 everyone!

@satya164

This comment has been minimized.

Copy link

satya164 commented Nov 20, 2018

We should alias props.x to match the prop named $x

Worth noting that it won't be possible to statically type this sort of thing.

We could technically add JSX namespaces to our babel plugin

Babel plugins can't add new syntax afaik. You'll need to fork Babylon to be able to add a non standard syntax.

@diondiondion

This comment has been minimized.

Copy link

diondiondion commented Nov 20, 2018

Great, thanks for chiming in @geelen. Still don't like the $ sign based default filtering, but as long as there's customisation via the rejectProps function, I'm all for it. 👍

The only case this probably doesn't solve is the as prop, it would be great to be able to tell a styled component to ignore & pass it on to the wrapped component. (See #2129 for details)

Any idea how that could be taken into account?

@hallister

This comment has been minimized.

Copy link

hallister commented Nov 20, 2018

I'm in agreement with @diondiondion's previous comment. I guess I'm lost on why it's clumsy to handle it at the component definition level. Is there really a situation where you might want to pass a prop sometimes but other times not? That sounds like an implementation issue.

But as long as customization exists via rejectProps (although, I'd argue filterProps or transientProps makes more semantic sense, as you're not rejecting them, you're just not passing them to the next level) I'll also jump on board. Hacky workarounds that either break syntax highlighting or throw errors for invalid DOM attributes are worse than anything suggested here.

@chengjianhua

This comment has been minimized.

Copy link

chengjianhua commented Dec 3, 2018

Recently I just found that using data-xxx as props name solved all my problem. It can pass down any numbers of layers and didn't cause React to emit warnings about non-DOM properties passed to dom elements.

Everything is ok, no extra props, no additional layers, no extra mechanism, leveraging the native attributes of HTML spec. Only one caveat that names are more verbose.

@dan-dr

This comment has been minimized.

Copy link

dan-dr commented Dec 18, 2018

@chengjianhua it may be valid but they bloat your html

I was trying to monkey patch SC so that I don't have to do this for every 3rd party component:

const StyledLink = styled(({ primary, ...p }) => <Link {...p} />)`
  color: ${p => primary ? 'blue': 'white'};
`

I came up with something that I think could be a good, non-breaking API:

// before first usage of styled
import styled from 'styled-components'
import omit from 'lodash.omit'

styled.transient = propsToOmit => {
  return tag => (...args) => {
    const Elem = styled(tag)``

    return styled(props => <Elem {...omit(props, propsToOmit)} />).call(
      styled,
      ...args
    )
  }
}

// After monkey patch usage
styled.transient(['primary'])(Link)`
   color: ${p => primary ? 'blue': 'white'};
`

I rarely monkey patch, and I actually don't need custom props omission very often, but I mainly want it because it makes my code much more readable than the non-monkey patched workaround.

It seems to be working quite well for me, and allows me to to gradually change my code.

What I like about it:

  • Non breaking - it's just a new function api
  • Declarative - you have to specify what you don't want to pass
  • Dynamic - can be implemented to receive a function so you can do cool stuff
  • Composable - you can create your own blacklist and reuse it, and it could probably be written in a way that you can compose them. for example:
// blacklist.js
export const global = ['my','list','of','custom','props']
export const buttons = ['primary', 'flat']

// styled-common.js
import { global, buttons }  from './blacklist'
import styled from 'styled-components'

const patchedStyled = styled.transient(global)
const buttonStyler = patchedStyle.transient(buttons) // not working in my monkey version but I'm guessing if implemented into sc it can be done

export { patchedStyled as styled }
export { buttonStyler }

// Component.js
import { styled, buttonStyler } from  './styled-common'

styled(Link)`` // will not pass down 'my','list','of','custom','props' props

buttonStyler(Button)`` // will not pass down the above props and 'primary' or 'flat' props
@joemaffei

This comment has been minimized.

Copy link

joemaffei commented Jan 21, 2019

@dan-dr I really like your solution. It's worth noting that lodash's omit method is fairly heavy (I measured it at 5.98kB today, Bundlephobia says it's 2.5kB). You might want to try a more lightweight implementation: https://gist.github.com/joemaffei/39976001f70d57b43ade0482994f2db2

@Jakobo

This comment has been minimized.

Copy link

Jakobo commented Feb 7, 2019

If you don't want to monkey patch styled, you can also create a HOC for consuming props. This just takes @joemaffei and @dan-dr's great work and moves it to a regular 'ole HOC.

const without = (...omitProps) => {
  const omitSingle = (object = {}, key) => {
    if (key === null || key === undefined || !(key in object)) return object;
    const { [key]: deleted, ...otherKeys } = object;
    return otherKeys;
  };

  const omit = (object = {}, keys) => {
    if (!keys) return object;
    if (Array.isArray(keys)) {
      // calling omitMultiple here would result in a second array check
      return keys.reduce((result, key) => {
        if (key in result) {
          return omitSingle(result, key);
        }
        return result;
      }, object);
    }
    return omitSingle(object, keys);
  };
  // HoF
  return C => {
    const WithoutPropsComponent = ({ children, ...props }) => {
      return React.createElement(C, omit(props, omitProps), children);
    };
    return WithoutPropsComponent;
  };
};

Used like this

// foo is consumed, and not passed on to MyMisbehavingThing
const MyStyledThing = styled(without('foo')(MyMisbehavingThing))`
  color: ${props => (props.foo === "bar" ? "#f00" : "#00f")};
`;

Here's a codesandbox that shows the HOC consuming & discarding the specified props.

edit: changed to HoF for compose() compatibility

@pumanitro

This comment has been minimized.

Copy link

pumanitro commented Mar 20, 2019

For a temporary solution we can do it as follows:

const SomeComponent = styled(({propIdontWant1, propIdontWant2, ...restProps}) => <ComponentIWantToUseOrHTMLElement {...restProps}/>)`
 background-color: ${props => props.propIdontWant1}
`;
@brandonkal

This comment has been minimized.

Copy link

brandonkal commented Jun 24, 2019

Why is this even something Styled Components or any library should be concerned with? This would be much better for React to handle. Pass everything down and have it filtered at the end. React wouldn't even need to have an arbitrary whitelist of valid DOM attributes. As a developer, you would prefix or namespace attributes you want to reach the DOM:

<Button primary dom:disabled />
OR
<Button primary $disabled />

This is extremely easy to lint.

Using a dollar prefix as a signal that "this is not a DOM attribute" works well. But it is awkward if it becomes a "this property is implemented as a SC/styletron/etc specific attribute". I'd much rather put a $ sign in front of all props that are not HTML attributes than have it be specific to any library. But at that point, it makes sense to go one step further and only namespace DOM attributes.

@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jun 24, 2019

@brandonkai we submitted a RFC to that effect in May of 2017, but it does not look like it will be implemented anytime soon. (facebook/jsx#81)

@samtimalsina

This comment has been minimized.

Copy link

samtimalsina commented Aug 23, 2019

For a temporary solution we can do it as follows:

const SomeComponent = styled(({propIdontWant1, propIdontWant2, ...restProps} => <ComponentIWantToUseOrHTMLElement {...restProps}/>)`
 background-color: ${props => props.propIdontWant1}
`;

This temporary solution worked for us too. There's a small typo here. Better written as:

const SomeComponent = styled(({propIdontWant1, propIdontWant2, ...restProps}) => <ComponentIWantToUseOrHTMLElement {...restProps}/>)`
 background-color: ${props => props.propIdontWant1}
`;
@tommedema

This comment has been minimized.

Copy link

tommedema commented Sep 24, 2019

Has anyone gotten these workarounds to work in a type safe manner with Typescript?

// props is `any`
const MyLabel = styled(({display, ...rest}) => <label {...rest}/>)`
  display: ${props => props.display ? 'inline' : 'none'}
`

// Type '{ display: boolean; }' does not satisfy the constraint...
// And: props: never
const MyLabel2 = styled<{ display: boolean }>(({display, ...rest}) => <label {...rest}/>)`
  display: ${props => props.display ? 'inline' : 'none'}
`
@3sanket3

This comment has been minimized.

Copy link

3sanket3 commented Sep 26, 2019

@tommedema It should be as below.

import React, { LabelHTMLAttributes } from "react";

const MyLabel2 = styled(
  ({
    display,
    ...rest
  }: LabelHTMLAttributes<HTMLLabelElement> & { display: boolean }) => (
    <label {...rest} />
  )
)`
  display: ${props => props.display ? 'inline' : 'none'}
`;

I lost my css intellisense though. Folks coding JS, is it the same for you or it's issue with TS only?

@mertindervish

This comment has been minimized.

Copy link

mertindervish commented Sep 27, 2019

@3sanket3

Sadly it's the same in JS. I managed to get my CSS IntelliSense back by assigning the "Transient" component into a variable as below:

import React, { LabelHTMLAttributes } from 'react'
import styled from 'styled-components'

const TransientMyLabel2 = ({
  display,
  ...rest
}: LabelHTMLAttributes<HTMLLabelElement> & { display: boolean }) => (
  <label {...rest} />
)

const MyLabel2 = styled(TransientMyLabel2)`
  display: ${props => (props.display ? 'inline' : 'none')};
`
@sami616

This comment has been minimized.

Copy link

sami616 commented Oct 4, 2019

Adding $ to props you don't want passed down seems like a really neat solution to me. Would be very handy for them to be aliased without the $ when interpolating like @geelen suggested. 😄

@brandonlilly brandonlilly referenced this pull request Oct 14, 2019
14 of 14 tasks complete
@dimitropoulos

This comment has been minimized.

Copy link

dimitropoulos commented Nov 1, 2019

couple things I want to add to the conversation:

Styletron has already been doing this for a long time and it works well!

The approach they use is:

All props starting with $ are filtered out and not passed to the underlying element.

I used styletron in quite extensively and I can say from experience that the approach taken in styletron (to use $) was both easy/obvious to work with for very junior and very senior developers alike. In fact the team never had any problems around this the way I have been having problems with (on a different project) the gotchas that come with styled-component's current implementation.

Maintaining a whitelist causes problems

It doesn't matter if styled-components maintains that list themselves, or depends on a 3rd party to maintain it.. you're always going to get problems due to "hey add my library's props too"-ism, as well as the ever-evolving HTML attributes spec adding common properties like importance and loading that are likely to be already in use in projects (and thus be a breaking change due to change in functionality when these new attributes are introduced). Let me say that again: since the browser can change at any time, we can expect that breaking changes will occur such that the same project (if using a props whitelist) will have different functionality even though they're using the same version of the library in question. See the thread below with Dan Abramov's tweet for ideas on how to prevent this.

Current approach

While styled-components used to maintain a hardcoded regex with all whitelisted properties, as of @probablyup's PR from August 2018 styled-components moved to use @emotion/is-prop-valid. There is a somewhat systemic problem with this, which I will describe below.

"Dan says so"

Dan Abramov warns not to use whitelists (styled-components is using a whitelist).

Please don’t use prop whitelists in components! This defeats the whole purpose of the DOM warning you were seeing.

This appeal to authority logical fallacy is brought to you by Dan's good reputation.

My use case

Recently I was upgrading an older project that used an older version of styled-components (before the move to @emotion/is-prop-valid). After styled-components moved to @emotion/is-prop-valid a PR was merged that added loading to the list of things passed through in the whitelist. This was the right thing to do technically speaking, since the DOM spec is what it is and we should support it. That said, as the commentators in that thread noticed (one of which is @probablyup, the author of this PR) thatL

this is kinda a breaking change (like any change to this whitelist, but the odds of breaking somebody's code in this case is quite huge in case of the "loading" prop)

Indeed, this sent me on a wild hunt to figure out why I kept getting this error because I was ignorant to this newly-added experimental HTML attribute. I had assumed (because I had seen loading used before with no problems in exactly the same context) that such a thing wouldn't present a problem. What's worse is that the debugging for something like this is extremely tedious:

67782849-bfd80600-fa3f-11e9-89a5-084abb19a240

Warning: Received `false` for a non-boolean attribute `loading`.

If you want to write it to the DOM, pass a string instead: loading="false" or loading={value.toString()}.

If you used to conditionally omit it with loading={condition && value}, pass loading={condition ? value : undefined} instead.
    in div (created by Context.Consumer)
    in StyledComponent (created by styled.div)
    in styled.div
    in Unknown (created by Poll)
    in div (created by Context.Consumer)
    in StyledComponent (created by styled.div)
    in styled.div (created by Poll)
    in Poll
    in Unknown
    in div (created by Context.Consumer)
    in StyledComponent (created by styled.div)
    in styled.div
    in Unknown (created by Context.Consumer)
    in Route (created by AppRouter)
    in Switch (created by AppRouter)
    in Router (created by HashRouter)
    in HashRouter (created by AppRouter)
    in AppRouter
    in Provider
    in ThemeProvider

In true form, there were many places in the app that used loading, which made this quite a bear to track down.

To compound the frustration, styled-component's documentation on this doesn't help my case because I was composing another component (i.e. styled(MyComponent)) where I wasn't able to pull out the props related to the error.

Once I figured out what was going on, which took me quite some time because I had to understand how the whitelist worked (there's no mention of whitelist in the styled-components codebase since it's just a dependency now and is-valid-prop isn't something obvious if you don't already know about it as we might expect someone encountering this problem might), I was able to fix it by changing loading to isLoading.


I wanted to highlight my use case because I think it's a good example of why using a whitelist can lead to:

  • constantly breaking changes (in a library outside of styled-components, no less)
  • extraordinarily tricky to debug for the average user of styled-components
  • non-obvious based on the same code working well in the past between upgrades (from what I can tell this wasn't mentioned in any styled-components release notes).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.