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

Use Vuepress #651

Closed
wants to merge 12 commits into from
Closed

Use Vuepress #651

wants to merge 12 commits into from

Conversation

kinow
Copy link
Member

@kinow kinow commented Apr 28, 2021

These changes close #640

Vuepress was working fine, until I got stuck with Webpack issues. Going down the rabbit hole, looks like updating to Yarn v2 could solve my current issue. It would also help for Cypress I think, so will give it a go tomorrow.

I tried to update it now following their migration guide, but the build failed. So there are probably things that need to be fixed in our build first (e.g. those many warnings I've been sweeping under the rag until now 😄 )

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Does not need tests (why?).
  • No change log entry required (why? e.g. invisible to users).
  • No documentation update required.

@kinow kinow mentioned this pull request May 3, 2021
12 tasks
@kinow
Copy link
Member Author

kinow commented May 5, 2021

Once #653 is merged I should be able to edit the last commit and make the build pass 👍

@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2021

Codecov Report

Merging #651 (bf9009f) into master (27b8c33) will not change coverage.
The diff coverage is n/a.

❗ Current head bf9009f differs from pull request most recent head 34b414b. Consider uploading reports for the commit 34b414b to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #651   +/-   ##
=======================================
  Coverage   90.98%   90.98%           
=======================================
  Files          84       84           
  Lines        1675     1675           
  Branches      105      105           
=======================================
  Hits         1524     1524           
  Misses        121      121           
  Partials       30       30           
Flag Coverage Δ
e2e 77.85% <ø> (ø)
unittests 81.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../components/graphqlFormGenerator/FormGenerator.vue 86.95% <ø> (ø)
src/components/graphqlFormGenerator/FormInput.vue 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27b8c33...34b414b. Read the comment docs.

@kinow
Copy link
Member Author

kinow commented Jun 1, 2021

Building OK now. But Vuepress still not quite working. First issue was with istanbul-instrument-loader, which can be replaced. I removed it temporarily to test it.

Then vue-markdown failed. This is more problematic, and we could have the same issue when upgrading to Vue 3 too. The issue appears to be that vue-markdown is bringing core-js 0.x or 1.x, while we use v3. It's not able to locate imported functions from core-js and just crashes.

I had a look at vue-markdown's code, and it's super simple. The issue is that it requires changes to their build process, but the project has had no activity in 4 years 😥 using the fork @adapttive/vue-markdown fixed Vuepress build, but now the Vuepress page is loaded blank, and there are errors in the console.

image

@kinow
Copy link
Member Author

kinow commented Jun 1, 2021

The core-js issue of Vue Markdown appears to be that Vue Markdown uses babel-runtime that is v6, while we are using Babel 7 miaolz123/vue-markdown#18 (comment)

@kinow
Copy link
Member Author

kinow commented Jun 1, 2021

The current issue now is in markdown-it-toc-and-anchor. A dependency of vue-markdown, but with last release in 2019 too 😥

@kinow
Copy link
Member Author

kinow commented Jun 1, 2021

Found one workaround for the global issue, by defining it globally and importing with require: vuejs/vuepress#1434 (comment)

@kinow kinow force-pushed the vuepress branch 2 times, most recently from f589182 to fb4c411 Compare June 1, 2021 09:38
@kinow
Copy link
Member Author

kinow commented Jun 1, 2021

Building passing, Vuepress displaying components correctly. Whew 😪

image

The "show code" part of the DemoAndCode component is not working as expected — or at least I couldn't make it work, and looking at the code I found it confusing why minHeight syntax is different 🤔 BuptStEve/vuepress-plugin-demo-code#35

@kinow
Copy link
Member Author

kinow commented Jun 1, 2021

Just need to add more components to the doc, show how it will look like in a GH pages, then it should be ready for review. Probably will finish next week or later, will move on to the GScan deltas now 👍

@kinow kinow self-assigned this Sep 12, 2021
@kinow kinow closed this Oct 4, 2021
@kinow kinow deleted the vuepress branch October 4, 2021 02:38
@kinow
Copy link
Member Author

kinow commented Oct 4, 2021

I have removed my branch, and closed the pull request, but moved the branch to this repo: https://github.com/cylc/cylc-ui/tree/vuepress

The reason is that it would take a bit longer to get everything working. And the everything is actually a lot of duplicated code.

The idea of Vuepress is really great. It compiles and runs your components in a documentation, similar to Sphinx in Python.

The main issue that I found, is that Vuepress does not load the project's configurations, especially not the webpack configuration of Cylc UI. Because of that, there are many configuration steps in the .docs/enhanceApp where I am basically duplicating our code from vue.config.js and babel.config.js.

This might become a problem for future developers since it is not necessarily an exact copy of the existing configuration. See the last commit, for example. I loaded Vuex with Vue.use(store), so that the Alert component could be displayed in the Vuepress docs.

However, that was not enough, since the this.$store is not loaded automatically as in Cylc UI. I had to troubleshoot a little, and then learn that Vue.mixin({ store: store }) was missing too. That's not mentioned in their docs, but you can find similar tips from forums & issues 😥

Definitely a great tool if other devs are willing to put up time and effort to maintain Vuepress in-sync and working. Main benefit is having the docs reflect exactly what we have in the source code (or fail if it is out of sync).

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.

Use jsdoc (or another tool?) to document .vue files
2 participants