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

feat: Vue components, mount options, global options, and v3 support #1409

Merged
merged 45 commits into from
Nov 23, 2020

Conversation

jhildenbiddle
Copy link
Member

@jhildenbiddle jhildenbiddle commented Oct 26, 2020

Fix #752 #1252

Summary

This PR significantly improves Vue functionality in docsify and addresses a few bugs/issues with the test environment.

Note that the preview site auto-generated for this PR is not built with the correct index.html so it will not process Vue content as expected. To preview locally, check out the branch/PR and run the following:

node ./test/config/server.js --start

Public Changes

  • Add vueComponents configuration option
  • Add vueMounts configuration option
  • Add vueGlobalOptions configuration option
  • Add Vue 3 support
  • Add styling for <output> element to vue.css
  • Update /docs/ site

Dev Changes

There was a significant test-related bug fixed in this PR. Due to the way Jest manages the JSDOM environment, global objects like the window and document objects are not reset after each test in the same file. As a result, event listeners added to the window object by docsify were not being cleared after each test, resulting in event listeners from previous previous tests continuing to be triggered.

  • Add support to docsify-init() for functions in config
  • Add .markdownlint.json
  • Add _logHTML.selector option to docsify-init()
  • Remove unnecessary global variables from test environment
  • Remove --runInBand flag from local Jest tests
  • Update Vue tests
  • Update Jest cleanup/reset in /test/config/jest.setup-tests.js
  • Fix waitForSelector() timeout in /test/helpers/wait-for.js
  • Fix docsify-init() handling of config.basePath option

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Repo settings
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Screenshot of new <output> styling (below code block):

Screen Shot 2020-10-26 at 2 42 11 AM

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

  • Yes
  • No

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

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

- Add detection of Vue HTML directives to trigger mounting
- Fix bug that processed markdown child nodes as global Vue instances even if vueGlobalOptions is not specified
- Fix bug that prevented passing vueGlobalOptions to global instances if `data()` prop did not exist
- Update Vue config check to test for existence of object keys instead of just the existence object
- Update tests
docs/vue.md Show resolved Hide resolved
docs/vue.md Outdated Show resolved Hide resolved
docs/vue.md Outdated
1. Process unmounted Vue content using `vueGlobalOptions`
- Docsify will not mount an existing Vue instance or an element that contains an existing Vue instance.
- Docsify will automatically destroy/unmount all Vue instances it creates before new page content is loaded.
- When processing `vueGlobalOptions`, docsify parses the child elements within the main content area (`#main`) and mounts the element if it contains Vue content. Docsify does not parse each individual node within the main content area.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not clear on the last bullet point. I guess if it'll start to explain this magic part, it might just need to go into more detail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree this description could use some work. To be honest, I debated about even adding it. I've updated the description to (hopefully) make it more clear. At the end of the day, this text exists to help explain why this edge case won't work:

window.$docsify = {
  vueGlobalOptions: {
    data() {
      return {
        globalMsg: 'Hello from vueGlobalOptions!',
      };
    },
  },
  vueMounts: {
    '#test': {
      data() {
        return {
          mountMsg: 'Hello from vueMounts!'
        };
      },
    },
  },
};

Example 1: This will work

<!-- Source -->
<p>{{ globalMsg }}</p>
<p id="test">{{ mountMsg }}</p>
<!-- Output -->
<p>Hello from vueGlobalOptions!</p>
<p id="test">Hello from vueMounts!</p>

In this example, docsify first mounts the top-level <p id="test"> from vueMounts. Docsify then iterates over the top-level child elements (<p>), skips the ones that contain or are themselves Vue instances, detects Vue template syntax in the top-level <p> element, and mounts it using vueGlobalOptions. Great.

Example 2: This will not work

<!-- Source -->
<div>
  <p>{{ globalMsg }}</p>
  <p id="test">{{ mountMsg }}</p>
</div>
<!-- Output -->
<div>
  <p>{{ globalMsg }}</p>
  <p id="test">Hello from vueMounts!</p>
</div>

Like the previous example, docsify first mounts the top-level <p id="test"> from vueMounts. Docsify then iterates over the top-level child elements (<div>), detects that <div> contains a Vue instance (<p id="test">) and skips that element. This behavior is explained in the following "Technical Notes" bullet point:

  • Docsify will not mount an existing Vue instance or an element that contains an existing Vue instance.

I think it is highly unlikely that this will be an issue, but I added the related technical notes in hopes of making life easier for everyone if it does. The trick is how to do this succinctly without going into the same level of detail above. Hopefully the changes I've put in place are good enough for now. We can always revisit if/when needed.

Copy link
Member

Choose a reason for hiding this comment

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

@jhildenbiddle The example of what works vs what doesn't will be very helpful to some people. I think it'd be worth pasting what you wrote into the docs.

-moz-osx-font-smoothing initial
-webkit-font-smoothing initial

.markdown-section code
Copy link
Member

Choose a reason for hiding this comment

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

Are these style changes potentially breaking (website will look different when v4 user updates)?

Copy link
Member

Choose a reason for hiding this comment

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

Oh never mind, looks like only refactoring.

Copy link
Member Author

Choose a reason for hiding this comment

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

Non-breaking change because styles did not exist previously.

@sy-records
Copy link
Member

Thanks to your contribution. the vue content in documentation now looks great 👍🏻

sy-records
sy-records previously approved these changes Nov 20, 2020
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.

LGTM.

Koooooo-7
Koooooo-7 previously approved these changes Nov 22, 2020
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.

damnnnn good. 👍

sy-records
sy-records previously approved these changes Nov 23, 2020
@jhildenbiddle jhildenbiddle merged commit bd662c4 into develop Nov 23, 2020
@jhildenbiddle jhildenbiddle deleted the feat/vue-options branch November 23, 2020 20:39
@sy-records sy-records mentioned this pull request Feb 5, 2021
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vuejs related to Vue.js
Projects
None yet
Development

Successfully merging this pull request may close these issues.

consider support vue data through config globally
4 participants