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

refactor: package changes #438

Merged
merged 35 commits into from
Apr 12, 2021
Merged

refactor: package changes #438

merged 35 commits into from
Apr 12, 2021

Conversation

danielroe
Copy link
Member

No description provided.

@danielroe danielroe requested a review from pi0 April 7, 2021 10:27
@danielroe danielroe marked this pull request as ready for review April 7, 2021 20:38
Copy link
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

Other than comments, LGTM 👍

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
src/babel-plugin.js Show resolved Hide resolved
src/globals.ts Outdated Show resolved Hide resolved
src/globals.ts Outdated Show resolved Hide resolved
src/module.ts Outdated Show resolved Hide resolved
@pi0
Copy link
Member

pi0 commented Apr 8, 2021

Some issues discovered with test deploy:

  • dist/index.d.ts is wrongly generated
  • unneded dist/globals.mock is generated
  • test script is not locally working (also triggers lots of warnings)
  • We could use a default export from runtime to properly instruct blindly-upgraded users to import from /module

@danielroe
Copy link
Member Author

danielroe commented Apr 8, 2021

@pi0 I think likely issues 1 & 3 would be fixed if you rerun yarn install (cb42db8). dist/globals-mock is needed for jest at moment because otherwise ~composition-api-globals can't be resolved.

See also #429 (re: adding @vue/composition-api to transpile array).

@pi0
Copy link
Member

pi0 commented Apr 8, 2021

dist/globals-mock is needed for jest at the moment because otherwise ~composition-api-globals can't be resolved

I think jest only expects export to existing since we are using exports field without a wildcard map. Building globals.mock to a temp directory might do the trick :)

See also #429

It is a little bit confusing since assumptions in that issue are based on before this PR for doing dedup by alias. Also as mentioned in commit comment, transpile is for transpile (not side-effect of inlining). I guarantee you it is right thing to do :)

@danielroe
Copy link
Member Author

@pi0 I mean we need to produce it for end-users who want to use jest testing:

'~composition-api-globals': '@nuxtjs/composition-api/dist/globals-mock',

@pi0
Copy link
Member

pi0 commented Apr 8, 2021

I mean we need to produce it for end-users who want to use jest testing

Is mentioned instruction for testing a vue component that uses @nuxtjs/composition-api but without nuxt builder? In that case seems okay but I think documentation needs better explanation for usage.

@danielroe danielroe requested a review from pi0 April 11, 2021 21:11
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.

2 participants