Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

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
@danielroe danielroe merged commit ddc9c0f into main Apr 12, 2021
@danielroe danielroe deleted the refactor/packaging branch April 12, 2021 12:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants