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 marked to 4.2.12 #1993

Merged
merged 7 commits into from
Apr 22, 2023
Merged

Update marked to 4.2.12 #1993

merged 7 commits into from
Apr 22, 2023

Conversation

dzsibi
Copy link
Contributor

@dzsibi dzsibi commented Feb 17, 2023

Summary

Updates marked to the latest version. There were a few pull requests by @snyk-bot (#1505, #1562, #1724, #1903, #1920), but due to some breaking changes in marked 4.0.0, it failed to build. I fixed the issues and all tests are green. I couldn't find any other breaking changes in terms of API and output, but since this is a jump of over 2 years worth of marked releases, it is possible that this will break something somewhere.

What kind of change does this PR introduce?

Other

Dependency upgrade

For any code change,

  • Related documentation has been updated if needed
  • Related tests have been updated or tests have been added

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

Related issue, if any:

#1885 is about the ancient dependency itself. #1874 provided a workaround (inject a newer version), but was likely due to the new extension type not working with the old marked version.

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

@vercel
Copy link

vercel bot commented Feb 17, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docsify-preview ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 22, 2023 9:54am

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 17, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5f9139f:

Sandbox Source
docsify-template Configuration

Koooooo-7
Koooooo-7 previously approved these changes Feb 20, 2023
Copy link
Member

@Koooooo-7 Koooooo-7 left a comment

Choose a reason for hiding this comment

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

Hi @dzsibi , nice work, thx ! LGTM.

@Koooooo-7 Koooooo-7 added dependencies Pull requests that update a dependency file attention labels Feb 20, 2023
@dzsibi
Copy link
Contributor Author

dzsibi commented Feb 20, 2023

I found one more breaking change related how the search plugin handles Table tokens (rows was renamed to cells, and it is now an object).

@Koooooo-7 Koooooo-7 self-requested a review February 21, 2023 03:05
@trusktr
Copy link
Member

trusktr commented Mar 17, 2023

I found one more breaking change related how the search plugin handles Table tokens (rows was renamed to cells, and it is now an object).

Nice find. We need to start a migration guide so we can document anything that breaks.

Whether or not we bump Docsify to 5.0 or keep it at 4.0 doesn't matter as much, as long as we can clearly have a written pathways for upgrade steps.

I think perhaps we can also link to marked change/migration log.

Requested ecosystem team to help testing this out on Discord so we can find any issues by running this on various projects using Docsify

@paulhibbitts
Copy link
Collaborator

paulhibbitts commented Mar 17, 2023

Thanks so much for this PR @dzsibi , I have started testing in my Docsify Starter Kits and Docsify-This:

✅ Open Course Starter Kit Test (https://paulhibbitts.github.io/test-docsify-open-course-starter-kit)
✅ Open Publishing Starter Kit Test (https://paulhibbitts.github.io/test-docsify-open-publishing-starter-kit)
✅ Docsify-This Test (https://paulhibbitts.github.io/test-docsify-this)

Will continue to test with various pages/configs and share back results.

UPDATE: Ok, I've been trying out the above builds during the day and so far have not spotted any obvious issues🤞🏼

@dzsibi
Copy link
Contributor Author

dzsibi commented Apr 3, 2023

Any progress on merging this / any additional input needed from me?

@Koooooo-7
Copy link
Member

Any progress on merging this / any additional input needed from me?

Hi @dzsibi , we are running this on some instances with our contributors to check if there has any breaking issues.
If everything is okay, we gonna merge it later.
Currently there is no more changes to do except we figure out if something unpredictable happens on this change.
Thx again. If we have some changes further required on this PR, we could work it with you then.

@paulhibbitts
Copy link
Collaborator

paulhibbitts commented Apr 5, 2023

My tests with several Docsify instances have all gone well, so now I've updated my main https://Docsify-This.net app with https://docsify-preview-mqvotzlmj-docsifyjs.vercel.app/lib/docsify.js and will report back in the next few days with the results - so far so good!

@paulhibbitts
Copy link
Collaborator

Ok, I've now run the test build for 5 days on https://Docsify-This.net and no issues have been reported nor have I seen any myself.

Copy link
Member

@Koooooo-7 Koooooo-7 left a comment

Choose a reason for hiding this comment

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

Since there is no other issues remained now. I gonna approve it on my side.
And we may create a new release for this individually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention dependencies Pull requests that update a dependency file priority : high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants