Skip to content

Migrate to Docusaurus 3.0 Completed With Checks#279

Merged
wetneb merged 14 commits into
OpenRefine:masterfrom
aakash-a-dev:aakash01
Jan 12, 2024
Merged

Migrate to Docusaurus 3.0 Completed With Checks#279
wetneb merged 14 commits into
OpenRefine:masterfrom
aakash-a-dev:aakash01

Conversation

@aakash-a-dev

Copy link
Copy Markdown
Contributor

Here, I completed the checkmark and also updated it to Docusaurus 3.0 although there is also 3.1.0 but still I updated to 3.0 for now.
Before checklist till making it Docusaurus 3.0:
7 errors

After checklist till making it Docusaurus 3.0:
All resolved

Hoping for a positive feedback and positive review for this PR.

Fix: #274

@netlify

netlify Bot commented Jan 9, 2024

Copy link
Copy Markdown

Deploy Preview for openrefine-website ready!

Name Link
🔨 Latest commit 2de6a15
🔍 Latest deploy log https://app.netlify.com/sites/openrefine-website/deploys/659ff64f2f20f700088e4757
😎 Deploy Preview https://deploy-preview-279--openrefine-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

I don't know why this happened, working again on this, but it run fine on my local machine
image

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

The error occurring due to node node v16.20.2 (npm v8.19.4), but we have to update the node engine to "node": ">=18.0", do we require to do this in code, or anyone have to setup this in Netlify side?

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

Here is it not accepting my PR, it due to Node engine 18, so I deployed it manually
https://659d918bd0b91930b5787c8a--spectacular-kringle-679929.netlify.app/
Waiting for your follow up on this.

@wetneb wetneb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @aakash-a-dev, thank you so much for having a go at this! Looking at the "Files changed" tab of this pull request, I see quite some Docusaurus example files which you seem to have committed into your pull request. Needless to say, I think those should not be included. The rest of the changes look promising, I would review them once you have removed those example files.

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

@wetneb Thank you for your comment, I went through it. Truly sorry for this. I rectified the example data files uploaded.

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

I also corrected the exporting.md when I went through the file changes few lines were unnecessary. Thank you for let me give my attention towards here.

@aakash-a-dev aakash-a-dev requested a review from wetneb January 10, 2024 07:43

@wetneb wetneb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks a lot!

Comment thread docs/manual/exporting.md Outdated
Comment thread docs/manual/wikibase/configuration.md Outdated
@wetneb

wetneb commented Jan 10, 2024

Copy link
Copy Markdown
Member

I have updated Netlify to use Node.js 20. Next time you make changes to this PR (for instance, solving the merge conflicts with yarn.lock), the site should compile properly.

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

[cause]: Error: Failed to compile due to Webpack errors.
Error: Docusaurus server-side rendering could not render static page with path /docs/manual/exporting because of error: Objects are not valid as a React child (found: object with keys {}). If you meant to render a collection of children, use an array instead.

Getting this error while building it in netlify. Any way I can rectify it.

@aakash-a-dev aakash-a-dev requested a review from wetneb January 10, 2024 09:34
@thadguidry

thadguidry commented Jan 10, 2024

Copy link
Copy Markdown
Member

I would upgrade to 3.1 then in this case? ... that way we can take advantage of the main thing folks are complaining about just like us... "broken anchors". They added a new checker for that in v3.1

🚀 New Feature
docusaurus-mdx-loader, docusaurus-module-type-aliases, docusaurus-theme-classic, docusaurus-types, docusaurus-utils, docusaurus
#9528 feat(core): make broken link checker detect broken anchors - add onBrokenAnchors config (@OzakIOne)

but not sure on do/dont on the syntax changes ? dunno, but @slorber mentioned custom header ids need a special syntax for MDX v2 support facebook/docusaurus#3321 (comment)

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

Now It passed every case!. It worked this time found out the issue.
In export.md on line no 92 ({{ }}) was getting considerate as react child component so put that into so now it worked with out any conflict.

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

@thadguidry I upgraded this to 3.1 only, when I went through documentation that's the best thing I thought to do. So the current code is for 3.1 only.

@thadguidry

thadguidry commented Jan 10, 2024

Copy link
Copy Markdown
Member

@aakash-a-dev but did you read the comment that l linked to? It specifically addresses @ostephens concern about the header ids, I think? But it looks like their official docs don't mention those things? Maybe we should ask them?

slorber commented [on Dec 14, 2022]?(facebook/docusaurus#3321 (comment))
Thanks for reporting @xiaogaozi

Note as part fo the MDX 2 migration (facebook/docusaurus#8288), {#headingId} is not a valid syntax anymore, and Titus suggests we use {/*#headingId*/} instead.

It should also be possible to escape ids: \{#headingId}

Make sure to adapt once we upgrade to MDX 2

@ostephens

ostephens commented Jan 10, 2024

Copy link
Copy Markdown
Member

@aakash-a-dev but did you read the comment that l linked to? It specifically addresses @ostephens concern about the header ids, I think? But it looks like their official docs don't mention those things? Maybe we should ask them?

It should also be possible to escape ids: /{#headingId}

@thadguidry This is the approach that @aakash-a-dev has taken so all good I think, although feedback to the Docusaurus documentation would make sense

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

@aakash-a-dev but did you read the comment that l linked to? It specifically addresses @ostephens concern about the header ids, I think? But it looks like their official docs don't mention those things? Maybe we should ask them?

It should also be possible to escape ids: /{#headingId}

@thadguidry This is the approach that @aakash-a-dev has taken so all good I think, although feedback to the Docusaurus documentation would make sense

I made it again /{#headingId} to {#headingId} I did it to rectify an issue but comes to know that the issue was taking place due to something else and I rectified it and mentioned in above comment. I guess we don't have to worry about escape ids now.

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

Hello I didn't heard from a long time on this PR, so just thought to put a follow up comment. If anywhere you think I require to make modification I am happy to do that. Thank you.

Comment thread docs/manual/cellediting.md Outdated
@wetneb

wetneb commented Jan 11, 2024

Copy link
Copy Markdown
Member

Could you fix the merge conflict with yarn.lock please? After that it should be good to merge.

@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

Resolved Yarn.lock conflict @wetneb

@aakash-a-dev aakash-a-dev requested a review from wetneb January 11, 2024 14:10
@aakash-a-dev

Copy link
Copy Markdown
Contributor Author

Pardon! I actually made the changes you asked and also worked on conflict error of yarn.lock file and every thing get done, is there any further changes I require to make? Will happy to that.

@wetneb wetneb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks a lot!

@wetneb wetneb merged commit 2bf42b8 into OpenRefine:master Jan 12, 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.

Migrate to Docusaurus 3.0

4 participants