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

convert Docsify and mixins to ES classes #1685

Merged
merged 1 commit into from
Dec 9, 2021
Merged

convert Docsify and mixins to ES classes #1685

merged 1 commit into from
Dec 9, 2021

Conversation

trusktr
Copy link
Member

@trusktr trusktr commented Dec 8, 2021

Summary

Converts Docsify and its internal mixins to ES classes. The advantage is static typing: this change begins to allow IDEs like VS Code to know the shape of classes for type checking and intellisense, so devs can go-to-definition on methods, or see if they are passing the wrong data type (still needs work, but this is a first step).

Docsify-classes.mp4

What kind of change does this PR introduce?

Refactor

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:

Tested in the following browsers:

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

@vercel
Copy link

vercel bot commented Dec 8, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/docsify-core/docsify-preview/27q5MnU4eLdPxqqJaJxKKvN8uFHm
✅ Preview: https://docsify-preview-git-classes-docsify-core.vercel.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 8, 2021

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 5e0f68c:

Sandbox Source
docsify-template Configuration

@trusktr
Copy link
Member Author

trusktr commented Dec 8, 2021

I had to run the test three times for them to pass. That's a bit annoying.

@trusktr
Copy link
Member Author

trusktr commented Dec 8, 2021

this is good to go

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.

I think It makes Docsify more readable with ES upgrade.
LGTM.

@trusktr
Copy link
Member Author

trusktr commented Dec 9, 2021

Btw, if you turn on "hide whitespace" in the "files changed" tab, the diff is easier to see. Not too many actual changes besides whitespace indentation.

Copy link
Member

@sy-records sy-records left a comment

Choose a reason for hiding this comment

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

If it doesn't affect previous users, it's fine.
LGTM.

@trusktr
Copy link
Member Author

trusktr commented Dec 9, 2021

If it doesn't affect previous users, it's fine. LGTM.

Yep yep! Let's keep as much backwards compatibility as possible. The new Solid-SSR stuff will be compatible with v4

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