-
Notifications
You must be signed in to change notification settings - Fork 138
Migrate from Vue 2 to Vue 3 #2622
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
Conversation
compileTemplate and renderToString are now included as dependencies of the main vue package, alongside API changes to methods provided. Bundled vue js file has also been updated.
Update syntax as well as dependencies, libraries to be Vue 3 compatible
Rebuild markbind bundles as well as vue bundles Vue now needs to be externalized for both client and server
7e3f433
to
6bcf6db
Compare
Further update core-web package. Vue needs to be externalized separately for server ('vue') and web-client ('Vue'), or it fails for case-sensitive systems e.g. Ubuntu. Also update index.js to use compiled render function and use updated Vue application mounting API.
When question component not used in quiz, certain buttons are rendered on server-side but not on client. Change the visibility of these buttons to hidden, but maintain in the DOM to prevent hydration mismatch.
Vue testcases overhauled due to changes in API. Additionally, improvement can be made to vue testcases in the future as snapshots make it hard to diagnose if change is intended or unexpected bug causing side effect.
Site is live and running on Vue 3! Preview at: https://deploy-preview-2622--markbind-master.netlify.app/ Go to the console and type: Ready for review - in the meantime, will try to author a separate PR to aid with PR-review. |
Anyways, thanks @gerteck for continuing this feature. Some of the solutions to the problem I had were ingenious, to say the least. I will try to review and comment on the core SSR flow but likely need someone to help with a in depth check on the other parts like whether all the vue components are functioning. CC @lhw-1 |
@gerteck great work 🚀
Please see video comparing the deployed preview and current master Screen.Recording.2025-03-11.at.9.35.50.PM.mov |
Thank you for your help on this PR! I also noted that popovers also lost their spacing: It's could be because the vue component libraries were updated or some differences in the configuration syntax, since I made minimal changes enough just to satisfy any errors and warnings. After investigation, I have found that this is because of whitespace preservation in the rendering of the HTML.
The gaps between each button is a result of the newline being parsed as whitespace between each button. However, Vue 3 automatically condenses and removes this whitespace, causing the resulting removal of the gap.
This will be added during the rendering process on server side to restore previous Vue 2 default behavior. |
Thank you @EltonGohJH for making time to look through the PR! Would not have been able to work on this without your previous work on it. Agreed on the suggestion. |
Currently I forked the CS2103 website and deployed it on my own gh-pages as like a quick draft to view it running on Vue 3 for now, to check for any major regressions first.
I didn't update the Agolia plugin API Keys and stuff so it will redirect to the original site. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried serving the markbind docs locally with this branch and got this warning:
due to
this is generated code so probably out of our control, but am wondering if we have a way to disable check for vue server-renderer just for this part.. |
$children has been in depreciated in Vue 3, causing logic in dropdown to stop working. Replace logic by using inject and provide by maintaining a list of submenus in the dropdown manually.
bind() should be migrated to mount()
Just Pushed a fix for the I initially updated the directive's I've now fixed this by updating the hook from beforeMount() to mounted(), ensuring all DOM manipulations and event bindings happen after the element is fully available in the DOM. The |
Add detail on custom Vue directives used.
Links for convenience:
Edit: I am including this for easier reference in latest comment Changelog (Beyond initial PR changes):
To Do:
Issues outside scope of this PR:
|
@gerteck This is a case of using a feature called |
Ah, same observation as @Incogdino. Left is on vue 2, right is on vue 3. |
I've taken a closer look at both of the CS2103T site deployments, and apart from the image size issue and the previously mentioned site-nav and page-nav whitespaces (which we can tackle outside of this PR, as per comment), there does not seem to be major issues. As discussed with @damithc, we're pushing for this to be merged in within the next 2 days, after which we will release MarkBind v6.0.0. @MarkBind/active-devs let's proceed with the final round of checks and testing, and @gerteck do let us know your plans for this PR if there are any remaining. |
Thank you for your help! I am looking into the image size issue, I have isolated and identified the regression, and can push a fix in the next hour or so. Otherwise, my plan would be that any other changes. if not major or functionality breaking can be left to be fixed after the PR has been merged. Edit:
|
For anyone who wants to understand the latest commit (and preceeding commit): The pictures did not resize and defaulted to max size was because the method to set height and width was not firing, which were set to fire on The reason the load event fired in Vue 2 but not in Vue 3 is likely due to a difference in hydration behavior between Vue 2 and Vue 3 during SSR (Server-Side Rendering). In short, it seems like:
This seems to be a somewhat known (?) issue through search. It could be that that when the component is hydrated in Vue 3, the image is already in the DOM and is marked as loaded and not fire What I’ve done is to add a This
Thumbnails now fixed: Sizing of pics also now fixed: |
Additionally, update binary and test shots
Hi @gerteck, I have noticed a little differences under Admin Info/Ip/Week2. Difference 1Vue 3: https://gerteck.github.io/cs2103-website/admin/ip-w2.html Difference 2
|
Thanks @IanCheah for your help! I will have to investigate the tabs issue. Regarding the mermaid rendering, it seems related to CORS and seems to be quite flaky, it could have been a regression, or rather, side effect from my previous PR (#2614) shifting mermaid to load on demand for efficiency. I can only replicate the issue sometimes. It seems that some investigate on that is due as well, though I suppose if we support offline loading (#2548) and bundle it together it should totally resolve the issue - since it is likely due to CORS. Sometimes it works: But I also encountered when I had the CORS issue as well. But the flakyness would definitely explain why it was not spotted earlier. Could be issue on mermaid CDN (header issue, CORS)? I think we should postpone this issue till after the migration, and set up a separate issue to tackle this? Otherwise, any more commits will continue to balloon this PR. What do you think? This is such active devs may work on it in parallel, and if there are any breaking functionalities that we have not found, the previous markbind version can be used to generate existing websites. |
Yep I agree! Adding more commits may balloon the pr and even potentially introduce unforseen errors. We can set up a separate issue once the pr is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all your work, @gerteck!
This has no doubt been a massive undertaking on your part, and your efforts in documenting all the steps are greatly appreciated - I'm sure future devs will benefit from your efforts. Thanks to @EltonGohJH who started this effort 2 years ago, as well as to all the devs who helped to review and test this PR as well (@tlylt, @Tim-Siu, @Incogdino, @IanCheah, and of course @damithc)!
LGTM, and if there are no other updates, we can proceed with the planned release tonight 👍
As discussed, the release will be after the Mermaid rendering PR
Yup, that should be fine. But it needs to be fixed before releasing V6, as it breaks current usages. |
I second that 💯 👍 |
@lhw-1 Each PR must have a SEMVER impact label, please remember to label the PR properly. |
What is the purpose of this pull request?
Overview of PR:
Fixes #2597 - Steps Needed for Vue Migration.
MarkBind (v5.5.3) is currently using Vue 2. However, Vue 2 has reached EOL in 2023 and limits the extensibility and maintainability of MarkBind, especially the vue-components package, and external dependencies for Vue 2 components that are no longer actively maintained. (UI Library Package). This PR migrates MarkBind from Vue 2 to Vue 3. This builds on extensive efforts from #2224 - Vue Migration attempt 2 years ago by @EltonGohJH, which has helped immensely to set the foundation for this PR.
There are no functional changes to how MarkBind behaves:
Overview of changes:
Some basic understanding of both Vue 2, Vue 3 and a good understanding of MarkBind (in particular with regards to SSR, Hydrating, Bundling) is needed to review? the PR. I have previously linked some hopefully helpful resources in issue #2597 to help in understanding this PR.
The Vue migration involves major changes in 3 out of 4 packages of Markbind, namely the
core
,core-web
, andvue-components
. Changes incli
are only functional snapshot tests. Changes have been separated into commits that are aim to be modular and digestable.Commit History Overview
Commit:
Adapt processing logic to use Vue 3.3.11
- vue is updated for the core library.Commit:
Migrate Vue Components
- I update the configurations for Vue Components to Vue3 (test, lint).vue-components
package (vue-style-loader
,jest-env-jsdom
).floating-vue
,vue-final-modal
,portal-vue
). Every component or configuration dependency updated probably has some separate form of migration guide online.Commit:
Update core-web package 1
- Update dependencies for core-web (bundling). Update configuration for webpack.Commit:
Update core-web package 2
- Update the essential webpage js logicindex.js
in core-web.Commit:
Fix Hydration Issue for Question component
- Fixed issue that I did not detect previously, related to commit 2.Commit:
Update Vue Testcases and Snapshots
- As per title. I also realized ( Some Vue components tests are erroneous #2610 ), so I amended the erroneous tests. No point keeping useless tests.Commit:
Merge master
: PR updated with necessary changes from pre-requisite PRs.Update license txt file as per MIT License
Commit:
Update CLI Snapshots
- Snapshots for test generated sites are regenerated and commited.*.renderjs
), it is not meaningful. I realized that the render function is unnecessarily compared. This causes a lot of issues like pollution, unnecessary increased test load. I will see if I can make a separate PR to address this issue to increase developers quality of life. Related to Test site improvements #1637.Congratulations for getting this far. There's no other unrelated initial changes. (Further changes and bug fixes are detailed in comment history below, or resolved comments in code review)
Changelog in comment history:
Issues Fixed
Fixes #2597 - Steps Needed for Vue Migration
Fixes #2084 - Investigate Vue Migration
Fixes #2610 - removed irrelevant ineffective tests that don't work.
Fixes #2171 - reloading logic abstracted into
savePageRenderFnForHotReload
method inPageVueServerRenderer
, to reduce coupling fromPage/index.js
.Fixes #1536 - scopedSlots have been replaced to slots as per Vue 3 migration
Others
Related PRs for Reference:
#2596 - Unformatted Vue Migration PR (Closed)
#1534 - Vue 2 SSR to Resolve FOUC
Related PRs (not needed for Reference):
Additionally, I have tried as best as possible to isolate any separable changes into separate PRs from this PR for ease of code review and understanding. It is not necessary to look through those changes. These PRs, which include both enhancement and code maintanence, include:
I don't think it is feasible to further break down this PR (tests would not work).
Additionally, during the migration process, I have identified some issues, such as #2610, #2612, #2613, #2615, #2623, #2627, #2629, #2630, #2631, #2624. Most of these issues are not addressed in the scope of this PR, but a review of this PR might provide context to tackle these issues separately. Also, there are definitely regressions or changes due to change in dependencies, especially for the vue components. These can also be tackled separately in the future. Appreciate the help to identify any regressions if spotted and raising appropriate issues after the Vue Migration has been merged.
To-do: (Done)Testing instructions:
Some general tests I could think of:
Run Tests
npm run test
forcore
,cli
, andvue-components
.Build & Serve Sample Site
markbind init
,markbind build
,markbind serve
.Check SSR & Hydration
Plugin Compatibility
Cross-browser Check
Proposed commit message: (wrap lines at 72 characters)
Migrate from Vue 2 to Vue 3
Vue 2 has reached end-of-life in 2023, limiting maintainability,
extensibility of MarkBind. This commit migrates the codebase
(core, CLI, and vue-components) to Vue 3.3.11.
The migration includes major updates to server-side rendering
logic, hydration, component syntax, bundling setup (Webpack),
and test configurations. Vue 2 dependencies and deprecated
APIs have been replaced accordingly. Snapshot tests and test
site output have also been updated to reflect new Vue version.
This migration improves future extensibility and compatibility
with modern tooling and Vue ecosystem packages.
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major
,r.Minor
,r.Patch
.Breaking change release note preparation (if applicable):
This release deprecates support for Vue 2 and introduces full support for Vue 3.
Summary of Breaking Changes:
Vue 2 is no longer supported. All components and internal rendering logic now use Vue 3.3.11. Vue 2-based plugins and custom components must be updated to comply with Vue 3 syntax and API. Deprecated HTML5 syntax support is removed, such as
<font>
,<big>
, and similar tags that are no longer rendered or styled reliably.Scoped slots API (scopedSlots) has been replaced by the new slots API as per Vue 3 standards. Internal test and SSR setup has been restructured to reflect modern Vue 3 testing libraries and bundling practices.
What Site Authors Need to Do:
If you are using custom Vue components and directives, please migrate them using the Vue 3 Migration Guide. If your site uses deprecated HTML tags, replace them with semantic HTML5 alternatives or appropriate CSS styling. If you have extended MarkBind through plugins that rely on Vue APIs, refer to updated documentation for best practices post-migration.