Skip to content

Upgrade to Docusaurus V3#486

Merged
sumitkumar25 merged 7 commits intomainfrom
qian/upgrade-docusaurus
Dec 5, 2024
Merged

Upgrade to Docusaurus V3#486
sumitkumar25 merged 7 commits intomainfrom
qian/upgrade-docusaurus

Conversation

@lee00678
Copy link
Collaborator

@lee00678 lee00678 commented Nov 21, 2024

This PR addresses the dependency vulnerabilities by upgrading to docusaurus v3.

It's a pretty big change, it includes some root folder package.json update as well. In order to use react and reactdom@18 (required for v3), we need to remove packages/website/package-loc.json from install:clean script. This way we can ensure the build uses react@18.

defaultProps in our mdx file will cause an error/warning message. This is caused by MDX scanning the docs and look for words like defaultProps, we don't actually use them in our doc, they are just javascript objects. So renamed the example props.

Added a new authors.yml file under releases, this way we no longer to copy over all the author information for each release article.

To test: In the root folder do a npm run install:clean, npm run build, then run npm run website.

@lee00678 lee00678 marked this pull request as draft November 21, 2024 22:34
Copy link
Contributor

@rokotyan rokotyan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! You may also want to update the Contributing section to mention Solid.

image image image image

@zerollup/ts-transform-paths perpetual MIT 1.7.18 Stefan Zerkalica zerkalica@gmail.com
react perpetual MIT 16.14.0 n/a
react-dom perpetual MIT 16.14.0 n/a
react perpetual MIT 18.3.1 n/a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprised to see the React version change without any package.json updates

Copy link
Collaborator Author

@lee00678 lee00678 Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, good question. I think, based on my understanding, the react version gets hoisted to the parent level, so because of the later version in packages/website/package-lock.json, this become a later version.

x: d=>d.x,
y: d=>d.y,
});
export const MyComponent = ({ type = "XYWrapper", n = 10, name = "StackedBar", x = (d) => d.x, y = (d) => d.y, data=generateDataRecords(n), ...customProps }) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea, but I find the name MyComponent sound a little bit too generic. Maybe DocsStackedBar instead?

@rokotyan
Copy link
Contributor

defaultProps in our mdx file will cause an error/warning message. We need to start using defaultParams. I updated only the StackedBar.mdx as a test, once we agree on the approach, I will update the rest of the files.

@lee00678 Can you tell more about the defaultProps problem? What kind of error / warning?

@lee00678 lee00678 force-pushed the qian/upgrade-docusaurus branch 3 times, most recently from 81eb7d3 to 741ccf7 Compare November 25, 2024 19:20
@lee00678
Copy link
Collaborator Author

lee00678 commented Nov 25, 2024

defaultProps in our mdx file will cause an error/warning message. We need to start using defaultParams. I updated only the StackedBar.mdx as a test, once we agree on the approach, I will update the rest of the files.

@lee00678 Can you tell more about the defaultProps problem? What kind of error / warning?

@rokotyan This is warning we are getting caused by defaultProps. It occurs on pages where defaultProps are used. Component pages, for example.
Screenshot 2024-11-25 at 12 21 47 PM

@rokotyan
Copy link
Contributor

@lee00678 I believe this error comes from React and is related to React's defaultProps, not to our defaultProps` which are just simple JavaScript objects.

The only place where Unovis uses it is here:

VisTimeSeries.defaultProps = {
margin: { top: 10, bottom: 10, left: 10, right: 10 },
autoMargin: true,
padding: {},
x: (d: VisTimeSeriesDatum) => d.timestamp,

But this component part is not being used anywhere. I thought of releasing something like this to give our users slightly more complex components, but now I don't think it's a good idea (I would rather share it as an example).

I've tried commenting it out but still seeing the warning, so maybe some our our or Docusaurus dependencies uses defaultProps

@lee00678
Copy link
Collaborator Author

lee00678 commented Dec 2, 2024

Looks good! You may also want to update the Contributing section to mention Solid.

Resolved in a separate PR: #491

@lee00678
Copy link
Collaborator Author

lee00678 commented Dec 2, 2024

@lee00678 I believe this error comes from React and is related to React's defaultProps, not to our defaultProps` which are just simple JavaScript objects.

The only place where Unovis uses it is here:

VisTimeSeries.defaultProps = {
margin: { top: 10, bottom: 10, left: 10, right: 10 },
autoMargin: true,
padding: {},
x: (d: VisTimeSeriesDatum) => d.timestamp,

But this component part is not being used anywhere. I thought of releasing something like this to give our users slightly more complex components, but now I don't think it's a good idea (I would rather share it as an example).

I've tried commenting it out but still seeing the warning, so maybe some our our or Docusaurus dependencies uses defaultProps

I actually think this issue maybe simply caused by the fact we are calling them defaultProps, because for Line.mdx, we call it lineProps and the page is not erroring out. We could update the doc to avoid defaultProps and set to scatterProps etc. Since what we have is javascript object, maybe this is a better solution?

@rokotyan
Copy link
Contributor

rokotyan commented Dec 2, 2024

@lee00678 Maybe, let's try it

@lee00678 lee00678 marked this pull request as ready for review December 2, 2024 19:54
@lee00678
Copy link
Collaborator Author

lee00678 commented Dec 2, 2024

@lee00678 Maybe, let's try it

That seems to fix all the defaultProps warnings.

@rokotyan
Copy link
Contributor

rokotyan commented Dec 2, 2024

@lee00678 Great! I didn't have any other comments so I think we can go ahead and merge this

@lee00678 lee00678 requested a review from sumitkumar25 December 3, 2024 00:02
@lee00678 lee00678 force-pushed the qian/upgrade-docusaurus branch from 237bc7b to 7be54cd Compare December 3, 2024 21:14
sumitkumar25
sumitkumar25 previously approved these changes Dec 3, 2024
sumitkumar25
sumitkumar25 previously approved these changes Dec 4, 2024
@sumitkumar25 sumitkumar25 merged commit 5eda7bf into main Dec 5, 2024
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.

3 participants