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

feat: allow to ignore native dependencies #5524

Merged
merged 6 commits into from
Jun 3, 2021

Conversation

farfromrefug
Copy link
Contributor

Along with that we simply need to have a cli parameter and an option in nativescript.config
@rigor789 could you help with the cli params? not sure where to do that.

Also i think we could even add ignored dependencies through the webpack config right? I dont remember how but we can modify the project data from there?

With this people can simply make it so that some deps are only used in dev (like vue-dev-tools and socketio) while not being present in production.

You can also make different releases for Google Play and Fdroid (what i am going for) where one would use non open source libs and the other would not (flavors on android)

Along with that we simply need to have a cli parameter and an option in nativescript.config
@cla-bot cla-bot bot added the cla: yes label May 7, 2021
Copy link
Member

@rigor789 rigor789 left a comment

Choose a reason for hiding this comment

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

LGTM, but to stay consistent with the other "types of dependencies", I'd rename the property to ignoredDependencies

lib/controllers/prepare-controller.ts Outdated Show resolved Hide resolved
lib/definitions/project.d.ts Outdated Show resolved Hide resolved
lib/services/plugins-service.ts Outdated Show resolved Hide resolved
lib/services/project-changes-service.ts Outdated Show resolved Hide resolved
lib/tools/node-modules/node-modules-builder.ts Outdated Show resolved Hide resolved
Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
@cla-bot
Copy link

cla-bot bot commented Jun 1, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @farfromrefug.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@cla-bot cla-bot bot removed the cla: yes label Jun 1, 2021
Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
@cla-bot
Copy link

cla-bot bot commented Jun 1, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @farfromrefug.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
@cla-bot
Copy link

cla-bot bot commented Jun 1, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @farfromrefug.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
@cla-bot
Copy link

cla-bot bot commented Jun 1, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @farfromrefug.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
@cla-bot
Copy link

cla-bot bot commented Jun 1, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @farfromrefug.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@farfromrefug
Copy link
Contributor Author

@cla-bot check

@cla-bot
Copy link

cla-bot bot commented Jun 1, 2021

The cla-bot has been summoned, and re-checked this pull request!

@cla-bot cla-bot bot added the cla: yes label Jun 1, 2021
@rigor789
Copy link
Member

rigor789 commented Jun 1, 2021

I'm thinking of adding the ignoredDependencies to the nativescript.config.ts. The other thought was package.json but we are trying to move away from too many custom "things" in there... The CLI flags, we probably can implement later if required.

I'm also thinking we should probably only ignore them in release/production builds - does that make sense, or is there a case where we'd like to ignore them in debug builds too?

@farfromrefug
Copy link
Contributor Author

@rigor789 I am ok with ignoring them only in production if used through nativescript.config.ts. But what i would really want is to have total control when defined through webpack config file. For example when using sentry i actually want to "ignore" it in dev too (and in f-droid production builds).
So the question is could i override it through webpack config?

@rigor789 rigor789 merged commit 6ba5cd6 into NativeScript:master Jun 3, 2021
@farfromrefug farfromrefug deleted the ignore_dependencies branch June 3, 2021 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants