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

Update default props #2926

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from
Draft

Conversation

AnnMarieW
Copy link
Collaborator

@AnnMarieW AnnMarieW commented Jul 22, 2024

closes #2919

I updated all the components listed in #2919 except for dcc.Location because it's still a class component.

  • validate if it works with the typescript components
  • I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

  • I have added entry in the CHANGELOG.md
  • If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follows
    • this GitHub #PR number updates the dash docs
    • here is the show and tell thread in Plotly Dash community

@AnnMarieW AnnMarieW marked this pull request as draft July 22, 2024 14:27
@gvwilson gvwilson added the infrastructure build process etc. label Jul 23, 2024
@gvwilson gvwilson assigned AnnMarieW and unassigned T4rk1n Jul 23, 2024
@AnnMarieW
Copy link
Collaborator Author

AnnMarieW commented Jul 23, 2024

Another dependency to update - our version of eslint does not seem to support the spread operator. eslint/eslint#10307

> eslint src scripts

/home/circleci/dash/components/dash-html-components/src/components/A.react.js
  12:52  error  Parsing error: Unexpected token ..

For example, this error is from defining the defaults like this in A.react.js

const A = ({n_clicks = 0, n_clicks_timestamp = -1, ...props}) => {

There is an error for each component.

@gvwilson gvwilson added feature something new P2 needed for current cycle and removed infrastructure build process etc. labels Aug 13, 2024
@gvwilson
Copy link
Contributor

@AnnMarieW can you please let us know if you're still working on this one or is it ready for review? thanks - @gvwilson

@AnnMarieW
Copy link
Collaborator Author

Hi @gvwilson

I'm afraid, I've hit a roadblock on this one. Here's the current status:

  • The components listed in issue 2919 are updated so that the defaults are defined in the function signature. I verified that the default props are set correctly in the component and the docstrings are generated correctly to show the defaults.
  • The sample typescript component is updated. I verified that the defaults are set correctly in the component, however the docstrings do not show the default values.

I found this project that parses the default props from the function signature in typescript components, however, I'm stuck trying to integrate that logic into extract-meta.js

If someone could help with that piece, I could probably finish the rest (ie tests) However, I'm traveling for a few weeks, and would be happy to turn this project over to someone if you'd like to get it wrapped up sooner.

@gvwilson
Copy link
Contributor

@T4rk1n please have a look at this one and help @AnnMarieW move it forward when you can. thanks - @gvwilson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new P2 needed for current cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

React 18 warning defaultProps will be removed from function components
3 participants