Jump to Vue 3 #551
Merged
Jump to Vue 3 #551
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
micgro42
force-pushed
the
next-jumpToVue3
branch
from
0ef6d97
to
312caa8
Compare
|
This is now green, except for the visual regression tests, but strangely, they seem to have caught something somewhat real? The little whitespace that used to be between components is now gone. o.O |
micgro42
force-pushed
the
next-jumpToVue3
branch
4 times, most recently
from
11054df
to
1558156
Compare
micgro42
force-pushed
the
next-jumpToVue3
branch
from
1558156
to
2a8ff15
Compare
This is a single big commit that migrates all the components in the next branch to Vue 3. With the options API, there were a lot of error of typescript not understanding where props are coming from when they were used in methods. It is unclear why that is, but it can be fixed by moving the code to script setup. Therefore that was done for the Button and the Lookup components. A further consideration is that Vue 3 now renders comments (at least in development mode) and so it breaks some tests that expects a single root. The tests of the LookupInput in particular needed some more work: * Vue.nextTick is now nextTick * disabled is now always the empty string if present * .native has been removed ** the native focus event has to be triggered on the actual HTML element Also, many more wrapper methods are now async so that we can just await them instead of having to wait for nextTick explicitly. This means that nextTick is no longer needed in the Lookup tests. A lot of the tests produced confusing typescript errors. The root cause of these mysterious errors turned out to actually be that we were still using the propsData key. Apparently that is deprecated when using javascript tests, but breaks hard when typescript is used. The tests in LookupCore.spec.ts specifically need also the change to src/shims-vue.d.ts to work, the others don't. Also there is a change in Vue3 in how the whitespace coming from newlines in the template between tags is handled. This affects us insofar as some of the whitespace we saw in storybook is now gone. This is fixed by adding a compiler option to those stories. This generally doesn't affect production because there we are not relying on that whitespace from newlines in the template, but rather create whitespace via CSS where we want it. Also, using props in the storybook stories to work with the Controls Storybook addon somehow stopped working. So some stories were changed to inject the args into the `data` option instead. The comments for the LookupInput events are moving to the same events in Lookup component, so that they actually show up in storybook. Typescript also validates that the values given to props match the excepted types. Though I saw these errors only in my IDE so far, not yet in the actual type check during npm run build. Bug: T303503
micgro42
force-pushed
the
next-jumpToVue3
branch
from
2a8ff15
to
86c398b
Compare
micgro42
requested review from
guergana,
lucaswerkmeister,
chukarave and
itamargiv
|
Let me know if there are any questions that are not answered in the commit message. |
chukarave
approved these changes
Mar 10, 2022
|
the two |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The components build now successfully, tests pass and storybook works.
The
Messagecomponent is still missing. It was accidentally removed as part of #534 and will be added back in an extra PR after this one is merged.