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

fix(expo): Do not publish plugin tests and sources to NPM #5565

Merged
merged 3 commits into from
Aug 3, 2021

Conversation

barthap
Copy link
Contributor

@barthap barthap commented Aug 3, 2021

Description

I noticed that the whole plugin directory is published to NPM. The only subdirectory really needed is plugin/build, others (src, tests) can be npm-ignored to make the package smaller.

The plugin/build dir is generated from plugin/src during npm prepare step. Its content is required by the app.plugin.js file.

Related issues

Release Summary

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

🤷


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@vercel
Copy link

vercel bot commented Aug 3, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/9FU1LbJ5hBqUJM9e2TcH1bjqLe17
✅ Preview: https://react-native-firebase-git-fork-barthap-plugin-e77c0f-invertase.vercel.app

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/DFp9YCMnHn8eGr8EPqgYUyrdzsMS
✅ Preview: Canceled

@codecov
Copy link

codecov bot commented Aug 3, 2021

Codecov Report

Merging #5565 (761209f) into master (6cb035c) will increase coverage by 3.62%.
The diff coverage is n/a.

❗ Current head 761209f differs from pull request most recent head 3a41e2c. Consider uploading reports for the commit 3a41e2c to get more accurate results

@@             Coverage Diff              @@
##             master    #5565      +/-   ##
============================================
+ Coverage     67.56%   71.17%   +3.62%     
============================================
  Files           199      106      -93     
  Lines          9659     4405    -5254     
  Branches       1510      941     -569     
============================================
- Hits           6525     3135    -3390     
+ Misses         2719     1173    -1546     
+ Partials        415       97     -318     

Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Ah, good catch. Normally I'm a big proponent of shipping source with the package (to enable "drive-by" PRs via patch-package and the like), but in a monorepo that's not really even possible, so it is just inflating the package size with no benefit I can see

@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Aug 3, 2021
@mikehardy mikehardy merged commit 6b5dca5 into invertase:master Aug 3, 2021
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Aug 3, 2021
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Aug 9, 2021
* [app] npm ignore expo plugin source
* [crashlytics] npm ignore expo config plugin src
* [perf] npm ignore expo config plugin src tests
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