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

chore: Cleaned up Vue code #2184

Merged
merged 5 commits into from Jul 6, 2022
Merged

chore: Cleaned up Vue code #2184

merged 5 commits into from Jul 6, 2022

Conversation

ErikCH
Copy link
Contributor

@ErikCH ErikCH commented Jun 23, 2022

Description of changes

  • Moved Jest to latest
  • Added in JSX - For future tests or primitives
  • Fixed issue with Code Editor error Default export of the module has or is using private name in any script setup file that has an interface

Issue #, if available

#2027

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are updated
  • No side effects or sideEffects field updated
  • Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ErikCH ErikCH requested a review from a team as a code owner June 23, 2022 17:36
@changeset-bot
Copy link

changeset-bot bot commented Jun 23, 2022

🦋 Changeset detected

Latest commit: e7941ae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@aws-amplify/ui-vue Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ErikCH ErikCH changed the title Vue Cleanup chore: Cleaned up Vue code Jun 23, 2022
@ErikCH ErikCH mentioned this pull request Jun 23, 2022
8 tasks
const wrapper = mount(Authenticator);
const wrapper = render(Authenticator, {
global: {
components,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To get vue to see all the components we have to add them in here. Since we are using unplugin-vue-components. And it only works with Vite.

@@ -5,7 +5,7 @@
"strict": true,
"jsx": "preserve",
"importHelpers": true,
"declaration": true,
"declaration": false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this prevent type generation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our typescript d.ts generation still occurs with our vite typeScript2 plugin settings.

// vite.config.js
    tsconfigOverride: {
        compilerOptions: {
          sourceMap: true,
          declaration: true,
          declarationMap: true,
        },

That way we still have typescript definition files being created.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! Can we leave a comment here for future readers who may not have context in to vite type generation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! I'll add a note

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@@ -53,15 +53,13 @@
"eventsource": "~2.0.2",
"fs-extra": "^10.0.0",
"glob-parent": "^6.0.2",
"jest": "^26.6.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We used to need to pin jest, because of an issue with different jest versions breaking different tests being run on our frameworks. However, our tests no longer need this.

And I won't be able to use the latest version of the vue3-jest library unless we update to jest 27.

@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:11 Inactive
@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:11 Inactive
@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:11 Inactive
@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:11 Inactive
@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:41 Inactive
@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:41 Inactive
@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:41 Inactive
@ErikCH ErikCH temporarily deployed to ci June 23, 2022 18:41 Inactive
Copy link
Contributor

@reesscot reesscot left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@slaymance slaymance left a comment

Choose a reason for hiding this comment

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

LGTM!

@ErikCH ErikCH merged commit a8b77d4 into main Jul 6, 2022
@ErikCH ErikCH deleted the vue-cleanup-jsx branch July 6, 2022 18:48
@github-actions github-actions bot mentioned this pull request Jul 6, 2022
@ErikCH ErikCH mentioned this pull request Jul 14, 2022
5 tasks
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.

None yet

4 participants