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

fix(docz-core): always use custom prop filter if it's provided (#1403) #1415

Merged
merged 1 commit into from
Mar 31, 2020

Conversation

tripphamm
Copy link
Contributor

Description

Closes #1403

I'm curious how y'all feel about considering this a "breaking change" vs just a "fix".

Technically the new behavior is different from the old behavior and folks who are using a custom propFilter might end up seeing more props after this is merged.

Here's a summary:

Old behavior (without a custom prop filter):

  • any props that don't have a parent are included
  • any props that have a non-node_module parent are included

New behavior (without a custom prop filter):

  • same as old behavior

Old behavior (with custom prop filter):

  • any props that don't have a parent are included
  • any props that have a non-node_module parent are include
  • any props that 1) have a parent and 2) parent is a node_module - will be run through the custom prop filter

This behavior seemed like a bug to me. The custom prop filter is almost always ignored and is only useful for white-listing some props that come from node_modules

New behavior (with custom prop filter):

  • all props are run through the custom prop filter

So, if folks were actually relying on the old "buggy" behavior of propFilter just being a node_modules whitelist, they will need to add some logic to their propFilter in order to re-create the old behavior.

Overall I believe that this new API is the right API, and it's just a matter of how to approach the change with the community, which I'll leave up to the maintainers

@tripphamm
Copy link
Contributor Author

CI is failing here:

/examples/typescript/src/components/Button.tsx
  53:3  error  Parsing error: Unexpected token

  51 | export interface ButtonProps {
  52 |   scale: 'small' | 'normal' | 'big'
> 53 |   kind: 'primary' | 'secondary' | 'cancel' | 'dark' | 'gray'
     |   ^
  54 |   outline: boolean
  55 | }
  56 | 

This doesn't seem like it's related to my change.

I saw this error during development as well. Curiously, adding semi-colons fixes the issue, but I wouldn't expect that they're needed.

@rakannimer
Copy link
Contributor

Can you rebase from master, I've fixed the CI issue

@tripphamm
Copy link
Contributor Author

Looks good now 👍

@rakannimer
Copy link
Contributor

Thank you !

@rakannimer rakannimer merged commit 53063ff into doczjs:master Mar 31, 2020
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.

docgenConfig.propFilter is ignored
2 participants