-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
chore: remove vue/compat
and complete Vue 3 migration
#7133
Conversation
Current Playwright Test Results Summary✅ 14 Passing Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 10/19/2023 04:08:24pm UTC) Run DetailsRunning Workflow e2e-couchdb on Github Actions Commit: 9581706 Started: 10/19/2023 04:03:04pm UTC Current Playwright Test Results Summary✅ 144 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 10/19/2023 04:08:24pm UTC)
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Basic Condition Set Use ConditionSet has correct outputs when telemetry is and is not available
Retry 1 • Initial Attempt |
0% (0)0 / 64 runsfailed over last 7 days |
1.56% (1)1 / 64 runflaked over last 7 days |
📄 functional/plugins/notebook/restrictedNotebook.e2e.spec.js • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Restricted Notebook with a page locked and with an embed @addinit Disallows embeds to be deleted if page locked @addinit
Retry 1 • Initial Attempt |
2.50% (1)1 / 40 runfailed over last 7 days |
40% (16)16 / 40 runsflaked over last 7 days |
Codecov Report
@@ Coverage Diff @@
## master #7133 +/- ##
===========================================
- Coverage 54.33% 41.96% -12.38%
===========================================
Files 651 414 -237
Lines 26187 12868 -13319
Branches 2535 0 -2535
===========================================
- Hits 14230 5400 -8830
+ Misses 11256 7468 -3788
+ Partials 701 0 -701
... and 506 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
vue/compat
and complete Vue 3 migration
@@ -77,16 +77,17 @@ const config = { | |||
MCT: path.join(projectRootDir, 'src/MCT'), | |||
testUtils: path.join(projectRootDir, 'src/utils/testUtils.js'), | |||
objectUtils: path.join(projectRootDir, 'src/api/objects/object-utils.js'), | |||
utils: path.join(projectRootDir, 'src/utils'), | |||
vue: path.join(projectRootDir, 'node_modules/@vue/compat/dist/vue.esm-bundler.js'), |
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.
Remove vue/compat
alias
} | ||
}, | ||
plugins: [ | ||
new webpack.DefinePlugin({ | ||
__OPENMCT_VERSION__: `'${packageDefinition.version}'`, | ||
__OPENMCT_BUILD_DATE__: `'${new Date()}'`, | ||
__OPENMCT_REVISION__: `'${gitRevision}'`, | ||
__OPENMCT_BUILD_BRANCH__: `'${gitBranch}'` | ||
__OPENMCT_BUILD_BRANCH__: `'${gitBranch}'`, | ||
__VUE_OPTIONS_API__: true, // enable/disable Options API support, default: true |
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.
Need to explicitly define these Vue compile time options, even if they're as defaults. Otherwise Vue generates a warning
@@ -20,7 +20,7 @@ | |||
* at runtime from the About dialog for additional information. | |||
*****************************************************************************/ | |||
|
|||
import Vue from 'vue'; | |||
import { nextTick } from 'vue'; |
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.
Vue
was only available to us because of vue/compat
. So now, only import the function itself
reorderEvent.newIndex, | ||
oldComposition[reorderEvent.oldIndex] | ||
); | ||
this.ladTableObjects[reorderEvent.newIndex] = oldComposition[reorderEvent.oldIndex]; |
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.
No need to use $set
anymore-- and it's been removed in Vue 3
@@ -78,6 +78,7 @@ | |||
<!-- Save Styles --> | |||
<toolbar-button | |||
v-if="canSaveStyle" | |||
ref="saveStyleButton" |
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.
Added these refs so we can access them in the unit tests instead of using $children
(which has been removed)
@@ -410,7 +410,7 @@ export default { | |||
this.manageEmbedLayout(); | |||
this.timestampAndUpdate(); | |||
}, | |||
convertMarkDownToHtml(text) { | |||
convertMarkDownToHtml(text = '') { |
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.
Added a default of empty string here as this was sometimes undefined
and causing issues. Still not sure why
class="c-search__input" | ||
aria-label="Search Input" | ||
tabindex="10000" | ||
type="search" | ||
:value="value" | ||
@input="handleInput" | ||
@click="handleClick" | ||
v-bind="$attrs" |
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.
need to bind $attrs
here as well since $listeners
was merged into it
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.
Nice work @ozyx! Did some cursory testing and looks great - really nice to not have any warnings!
Screen.Recording.2023-10-16.at.6.18.02.PM.mov
So many test updates!
I'd like to test this branch against VIPER before we merge this |
Closes #7131
Describe your changes:
vue/compat
and related compatibility settings in our webpack config.3.3.4
), completing the migration!Vue
in our unit tests$children
in our unit tests with$refs
All Submissions:
Author Checklist
Reviewer Checklist