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

Upgrade to Vue 3 #10298

Merged
merged 41 commits into from
Jan 15, 2025
Merged

Upgrade to Vue 3 #10298

merged 41 commits into from
Jan 15, 2025

Conversation

RayBB
Copy link
Collaborator

@RayBB RayBB commented Jan 8, 2025

Closes #7547

Overview

This PR upgrades Vue. Due to Vue's architecture, this requires upgrading all Vue-related dependencies simultaneously.

Changes Needed

  • Fixed styling issues in Library Explorer
  • Restored Merge UI functionality by adding async plugin globally (temporary solution approved by @cdrini)
  • Fixed the kebab_case function to work with acronyms
  • Update eslint config https://eslint.vuejs.org/user-guide/#configuration-eslintrc
    • This caused precommit to auto fix destroyed -> unmounted and beforeDestroy -> beforeUnmount
    • Make another ticket to upgrade the linter to use plugin:vue/vue3-recommended (which will make many automatic changes for best practices) use vue recommended sytles #10339
  • Updated the HelloWorld to be Vue 3

Technical Details

  • Considered externalizing Vue dependencies but kept current bundling approach since:
    1. We rarely (never?) have multiple Vue components per page
    2. It would work with a CDN but we don't want that
    3. Current build process is simpler to maintain
  • Minification is enabled via Vite's default settings

Implementation Notes

Testing Instructions

docker compose up
docker compose run --rm home make components
# visit http://localhost:8080/explore?ol_base=openlibrary.org

To test the dev setup run (outside of docker)

npm install --no-audit
COMPONENT="LibraryExplorer" npm run serve

TBH, I'm not really sure if this is needed but we already had it so I updated it.

Test the following components:

Stakeholders

@cdrini

confidence and access to the correct context to evaluate and use this code. -->

@RayBB RayBB changed the base branch from master to update-vue-import-extensions January 9, 2025 06:38
Base automatically changed from update-vue-import-extensions to master January 10, 2025 00:31
@RayBB RayBB marked this pull request as ready for review January 14, 2025 03:34
@RayBB RayBB changed the title Upgrade to Vue3 Upgrade to Vue 3 Jan 14, 2025
@RayBB RayBB mentioned this pull request Jan 14, 2025
14 tasks
@RayBB RayBB requested a review from cdrini January 14, 2025 03:40
@RayBB RayBB added the Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. label Jan 14, 2025
openlibrary/components/rollupInputCore.js Show resolved Hide resolved
Makefile Show resolved Hide resolved
<template>
<div class="page">
<h1>Hello Vuorld!</h1>
<script setup>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we might want to roll this back to the options API version ; we're still recommending the options API for Open Library for consistency with existing components and for being easier to use on simple cases -- which is most of Open Library's components.

@cdrini
Copy link
Collaborator

cdrini commented Jan 15, 2025

Wooohooo!!!! Great work @RayBB !!! This has stumped the team for multiple attempts 😁 This works perfect across all our components, and all the tech decision you've made here make perfect sense! Thank you!!

@cdrini cdrini merged commit e73034e into master Jan 15, 2025
8 checks passed
@cdrini cdrini deleted the vue3-2025-edition branch January 15, 2025 17:18
@cdrini
Copy link
Collaborator

cdrini commented Jan 15, 2025

image
😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Migrate to Vue 3
2 participants