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

Update npmignore and files package #248

Merged
merged 1 commit into from
Sep 27, 2019

Conversation

friederbluemle
Copy link
Contributor

A quick update to .npmignore and the files section in package.json that reduces the distribution artifact size by more than 70% (mainly by no longer including Gradle wrapper, which is not used by app consumers).

Before:

npm notice name:          react-native-keychain
npm notice version:       4.0.0
npm notice filename:      react-native-keychain-4.0.0.tgz
npm notice package size:  79.7 kB
npm notice unpacked size: 172.2 kB
npm notice total files:   31

After:

npm notice name:          react-native-keychain
npm notice version:       4.0.0
npm notice filename:      react-native-keychain-4.0.0.tgz
npm notice package size:  22.9 kB
npm notice unpacked size: 103.8 kB
npm notice total files:   22

CC @SaeedZhiany

@friederbluemle
Copy link
Contributor Author

Rebased.

@friederbluemle
Copy link
Contributor Author

@vonovak - For the merge, by any change could you use a regular merge? My commit is GPG signed, and I would prefer it not to be rewritten on the GitHub server (or having to force delete my local branch). I just rebased my branch onto the latest master.

@oblador
Copy link
Owner

oblador commented Sep 20, 2019

This doesn't include some pretty important files like the index.js. Did you test to use the package with these settings?

@friederbluemle
Copy link
Contributor Author

@oblador - index.js is defined as main in package.json. Along with a few other essential files, it is always included in the packed file. More info about this mechanism here: https://docs.npmjs.com/misc/developers#keeping-files-out-of-your-package
Yes, I tested this and you can easily do it too: Just run npm pack and observe the output :)

@oblador
Copy link
Owner

oblador commented Sep 27, 2019

OK, fair enough, i'll give it a try myself too then. Any reason we can't get rid of the .npmignore fully btw?

@friederbluemle
Copy link
Contributor Author

@oblador - Great question. Because of the typical setup of a RN library that contains nested app project the dependency resolution with Yarn may be tricky. There is some inconsistency in the way RN library example projects include the library they're showcasing (some use a NPM reference, some a relative path, some other babel/metro hacks). It is true that with the current setup the npmignore file could be removed. I have some local changes with other updates that might require the file to be re-added though. In order to avoid that I kept it for now. We can always fully remove it later. Please let me know if this is okay.
Rebased again.

@vonovak vonovak merged commit ae534a4 into oblador:master Sep 27, 2019
@vonovak
Copy link
Collaborator

vonovak commented Sep 27, 2019

Thanks for the explanation, I think this sounds good :)

@friederbluemle friederbluemle deleted the update-dist-files branch September 27, 2019 20:19
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

3 participants