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

Vue3 upgrade #4169

Closed
wants to merge 4 commits into from
Closed

Conversation

BoldBigflank
Copy link

@BoldBigflank BoldBigflank commented Sep 17, 2024

Upgrades to Vue3

Upgrades Storybook to Version 8

Updates ESLint rules

  • Changes to plugin:vue/vue3-recommended
  • Adds plugin:storybook/recommended
  • Adds eslint:recommended
  • Makes most existing errors into warnings
  • no-unused-vars is changed to an error with an exception to allow a function regex

NOTE: Currently Storybook loads without errors but many components are not looking like you would expect, so there's more to do to make a robust Storybook experience.

Copy link

netlify bot commented Sep 17, 2024

Deploy Preview for origin-betaflight-app ready!

Name Link
🔨 Latest commit a72634e
🔍 Latest deploy log https://app.netlify.com/sites/origin-betaflight-app/deploys/66e9e837e72a020008b196af
😎 Deploy Preview https://deploy-preview-4169.dev.app.betaflight.com
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@haslinghuis haslinghuis added the dependencies Pull requests that update a dependency file label Sep 17, 2024
@haslinghuis haslinghuis added this to the 11.0 milestone Sep 17, 2024
haslinghuis
haslinghuis previously approved these changes Sep 17, 2024
Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

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

LGTM ❤️

@nerdCopter
Copy link
Member

what CLI commands should i invoke for testing?

@BoldBigflank
Copy link
Author

@haslinghuis I just realized I used the wrong branch, I'm gonna have to redo my PR

@BoldBigflank
Copy link
Author

Wait, I misread and it wants the branch to NOT be named master, so we're good to go here.

@BoldBigflank
Copy link
Author

@nerdCopter yarn dev to load the site (things like the Betaflight logo have the right version info, Battery icon exists, etc)
yarn storybook will fire up the storybook host

@BoldBigflank
Copy link
Author

I get an error on loading the home page

bluetooth.js:99 Uncaught (in promise) TypeError: this.bluetooth.getDevices is not a function
    at BT.loadDevices (bluetooth.js:99:46)
    at new BT (bluetooth.js:57:14)
    at bluetooth.js:361:16

Can anyone confirm this is expected?

@haslinghuis
Copy link
Member

haslinghuis commented Sep 17, 2024

Not an issue with this PR.

Needs experimental flags being enabled #4024 (comment)

Obvious some code (likely porthandler) is bypassing

if (!this.bluetooth && window && window.navigator && window.navigator.bluetooth) {
this.bluetooth = navigator.bluetooth;
} else {
console.error(`${this.logHead} Bluetooth API not available`);
return;
}

@McGiverGim
Copy link
Member

High! Thanks for this PR, it's always welcome an update to more modern versions.
This PR is very long, including changes in " by ' (I don't know if they are mandatory/recommended for Vue3) so it's difficult to review fully but at least can be tested.
Some things don't work. For example, in options, if I enable the virtual mode, the combo doesn't reflect it without refreshing the page. In the current master it changes without problem. Could you verify this?

@haslinghuis
Copy link
Member

PortPicker is not following changes in options tab. Also baudrate default would be 115K2

image

@haslinghuis haslinghuis dismissed their stale review September 18, 2024 15:48

request changes

@BoldBigflank
Copy link
Author

BoldBigflank commented Sep 18, 2024

Here seems to be the reason that reactivity is missing, Vue 2 data object was deep reactivity, and Vue 3 is shallow reactivity:

https://v3-migration.vuejs.org/breaking-changes/data-option#migration-strategy

For users relying on the deep merge behavior from mixins, we recommend refactoring your code to avoid such reliance altogether, since deep merges from mixins are very implicit and can make the code logic more difficult to understand and debug.

I'm working through options to get this going again

@BoldBigflank
Copy link
Author

I haven't figured it out and will redo the PR when it's ready

@BoldBigflank BoldBigflank deleted the vue3-upgrade branch September 20, 2024 21:44
@BoldBigflank BoldBigflank restored the vue3-upgrade branch September 20, 2024 21:54
@BoldBigflank BoldBigflank deleted the vue3-upgrade branch September 20, 2024 21:59
@haslinghuis haslinghuis mentioned this pull request Dec 2, 2024
@nerdCopter
Copy link
Member

accurate, or just junk/noise: ?
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Tested
Projects
Development

Successfully merging this pull request may close these issues.

4 participants