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

Add a default to className #559

Open
wants to merge 1 commit into
base: 0.2.x
Choose a base branch
from

Conversation

ABuffSeagull
Copy link

@ABuffSeagull ABuffSeagull commented Apr 26, 2024

I was getting errors with className.split in my project.

Update: I realized this is cause I'm using the new React v19, which silently ignores prop-types.

@robmadole
Copy link
Member

I think I have this fixed in 0.2.1. Closing.

@robmadole robmadole closed this May 16, 2024
@WyvernDrexx
Copy link

WyvernDrexx commented May 17, 2024

@robmadole this is not fixed in version 0.2.1 please check. I am using React 18

The issue seems to be this, if the props has { className: undefined } as a result of passing through props of multiple components then, const allProps = { ...defaultProps, ...props } this would not have any affect on the props. Hence, the className.split()would throw error.

https://playcode.io/1874401

@WyvernDrexx
Copy link

Will something like this work:

const allProps = {}

  //We clean the undefined prop to use default prop values if present
  for (const propName in props) {
    const propValue = props[propName]
    if (typeof propValue === 'undefined') {
      allProps[propName] = defaultProps[propName]
    } else {
      allProps[propName] = propValue
    }
  }

@robmadole
Copy link
Member

Oh shoot. Yeah I get why that's happening @WyvernDrexx. We could probably do your trick or we could make a specific exception for just the className. Let me think on this and see if I can get another release out today.

@robmadole robmadole reopened this May 17, 2024
@WyvernDrexx
Copy link

WyvernDrexx commented May 17, 2024

@robmadole Much better approach would be this..

  const allProps = { ...props }

  /**
   * Loop through default props and replace any undefined user prop value with the value from defaultProps.
   */
  for (const prop in defaultProps) {
    const defaultValue = prop[defaultProps]
    if (typeof allProps[prop] === 'undefined') allProps[prop] = defaultValue
    else allProps[prop] = props[prop]
  }

This code will also clean all the undefined default props value and allProps will also have all defaultProps.

@jonny-dungeons
Copy link

Is there anything we are waiting on for this to be merged? Are there any reviewers available for this?

This is blocking my team and I on a project and unfortunately we have made the upgrade to 0.2.1.

@shehi
Copy link

shehi commented May 21, 2024

Same here. This should be fixed, especially when TS typing lists className prop as skippable:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants