Skip to content

Conversation

@gerteck
Copy link
Member

@gerteck gerteck commented Feb 6, 2025

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • DevOps
  • Improve developer experience
  • Others, please explain:

Related PRs for Reference:
#1534

Overview of changes:

MarkBind (v5.5.3) is currently using Vue 2. However, Vue 2 has reached EOL and limits the extensibility and maintainability of MarkBind, especially the vue-components package. (UI Library Package).

This PR tracks the Vue Migration Vue 2 to Vue 3.

Disgustingly intricate understanding of both Vue 2, Vue 3, low-level dependency APIs and MarkBind (in particular with regards to SSR, Hydrating, Bundling) is needed to conduct this.

Some relevant links to understanding / reviewing the PR:

Additionally, here are some relevant documentation of the codebase I have written and compiled to speed up the understanding process for purpose of peer development/review.

Note that currently, MarkBind Vue components are authored in the Vue Options API style. While migrating to Vue 3, we can continue to use this API style. Future work can consider cost benefit of switching to using Composition API.

will update desc further when i compile the information needed

left to do:

  • refactoring of vue components (continue using options API),
  • refactoring of plugins, wrt. Vue directives (breaking changes).
  • extensive update of tests afterwards
  • extensive documentation updates if and as needed

Fixes #2084
Fixes #2597
Fixes #2171 (Took the chance to fix w. changes in vue rendering APIs)

tbc..

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)

Migrate Vue 2 to Vue 3

tbc


Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

Reviewer checklist:

Indicate the SEMVER impact of the PR:

  • Major (when you make incompatible API changes)
  • Minor (when you add functionality in a backward compatible manner)
  • Patch (when you make backward compatible bug fixes)

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):

  • To be included in the release note for any feature that is made obsolete/breaking

Give a brief explanation note about:

  • what was the old feature that was made obsolete
  • any replacement feature (if any), and
  • how the author should modify his website to migrate from the old feature to the replacement feature (if possible).

Updated Vue Installation due to breaking changes
Updated dependencies for Vue and SSR logic
Updated SSR logic
@gerteck
Copy link
Member Author

gerteck commented Feb 9, 2025

https://vuejs.org/guide/extras/rendering-mechanism#templates-vs-render-functions

Considering use of templates instead of render functions as templates are simpler, and still conduct SSR. No pressing need to compile render functions from @vue/compiler-sfc actually.

Although based on previous PRs:

(we used to do CSR + ship the raw, uncompiled template which took very long to render)

The difference is that this PR currently takes the approach of SSR + ship raw, uncompiled template. (need to consider how long it will take to render)

Will need to do further investigation

Edit: OK nvm I managed to reimplemented Server Side Compilation of Render Functions after diving into the codebase for @vue/compiler-sfc

@tlylt
Copy link
Contributor

tlylt commented Feb 12, 2025

Hi @gerteck, thanks for taking this up! As you have probably read up on all the related past issues/PRs around the migration, I would just want to caution that please take into account of how other developers/reviewers can help in this migration process. If the work is not broken down into manageable chunks, it might be difficult review and assess the changes holistically.

@gerteck
Copy link
Member Author

gerteck commented Feb 13, 2025

I am currently facing issues which @EltonGohJH previously faced:

resolveComponent can only be used in render() or setup() error.
Occurs due to multiple Vue instances.? For SSR, the issue persists because two different Vue files are used (runtime-core.cjs.js and runtime-core.esm-bundler.js). This makes it non-trivial to solve, so SSR has been disabled for now. This is not trivial to debug.

Not too sure how to resolve this issue, but am looking into the matter.

Secondly, and related components that utilize portal vue also face the issue of Cannot read property 'isCE' of null in remote component with slot . Preliminary research suggests that this issue is also a result of having multiple Vue Instances.

@lhw-1
Copy link
Contributor

lhw-1 commented Feb 14, 2025

Just to add on a small comment, the first error message originates from here. Certainly not trivial to debug, and we'll need to look into if it is worth spending more time on the issue this semester.

Remove unnecessary updated dependencies to minimize scope
@lhw-1
Copy link
Contributor

lhw-1 commented Feb 18, 2025

As discussed, we'll have a couple of the active devs (CS3281) working on this issue - mainly by researching the topic and understanding what is causing the issue. We will set an internal deadline of 9 March. If by then we have not made significant progress with this, we will have to prioritize other issues / features.

Issue was due to multiple Vue instances where Vue was bundled in
the dev webpack causing nested vue components to not be rendered.

Marking vue as external should happen both on server and client.
Mostly updating depreciated syntax
Update dependencies, update configuration of webpack
Reinstate vue components
Skipped tests snapshot updated but continue to be skipped.
Error was due to multiple vue instances and package dependencies.
Shifted vue under root package.json, and set other vue
dependencies as peerDependencies.

This helps ensure that the version used is the same and
that the same instance is used.

Look at vuejs/core#4344 for related
discussion on the topic.
Vue no longer resolves to local node_module.
Add Vue as peerDependency of root package.json
Fix linting errors
Add stub to fix vue warnings.
Suspect previous error was something to do with package-lock,
where installing in root updated package-lock. Wasnt the
core issue.
Delete expected mermaid.html files till later migration
@gerteck gerteck mentioned this pull request Mar 10, 2025
13 tasks
@gerteck
Copy link
Member Author

gerteck commented Mar 10, 2025

I have reformatted and opened a new PR for the Vue migration #2622 . It has been structured to be as easy to follow as possible.

As advised by @tlylt , I have made best effort to break the work done into manageable chunks for reviewers and other developers.

I have isolated separable code changes into separate PRs as much as possible, which have since been merged.

  • (PRs: 2625, 2626, 2614).
  • The final PR2622 contains the minimal (Best Effort) amount of code changes to functionally migrate Vue 2 to Vue 3.

Additionally, in the PR and the description, currently, I have:

  • Modularized the commits - each commit deals with a logically separable component in the migration process.
  • Added an overview and detailed description for each commit - what is done with each commit, grouped logically.
  • Hyperlinked where convenient — Direct links to referenced commits, packages, external documentation to help in navigating quickly and access necessary context.
  • Added release note preparation, some suggested testing instruction, related issues for context at appropriate places and some other stuff.

Hope it makes it easier on PR reviewers! Will close this PR.

@gerteck gerteck closed this Mar 10, 2025
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.

Steps Needed for Vue Migration (Vue 2 -> Vue 3) Fix or simplify live reloading logic Investigate and list what needs to be done for Vue 3 migration

3 participants