Skip to content

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Dec 14, 2020

Purpose / Goal

postinstall scripts tend to be obnoxious when used for advertising, and there are better methods available. Usually if someone is interested in contributing, they'll seek out projects to contribute to, but postinstall scripts are shown on every build resulting in noisy logs that make it harder to debug failing builds.

The nature of postinstall means it can't be disabled without risking disabling an actual postinstall that is required for a particular package, and any form of check within the script (i.e for an env variable) won't prevent all the noise since the package managers always print the path to the scripts they're running before they run them.

Resolves #236

Type

Please mention the type of PR

  • [ ]Bug Fix
  • [x]Refactoring / Technology upgrade
  • [ ]New Feature

@coveralls
Copy link

coveralls commented Dec 14, 2020

Coverage Status

Coverage remained the same at 93.863% when pulling 77d13ad on G-Rath:remove-postinstall-script into 7081bd0 on NaturalIntelligence:master.

@guardrails
Copy link

guardrails bot commented Dec 14, 2020

⚠️ We detected security issues in this pull request:

Vulnerable Libraries (1)

More info on how to fix Vulnerable Libraries in Javascript.


👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

@amitguptagwl
Copy link
Member

@G-Rath methods you suggest are for only funding request. And the post install message I set is about my profile page link (as I remember correctly) which is not the right fit to "npm fund". From the build perspective, I believe it is just a single line in your logs.

You can understand that I have already spent a lot of time to this project. And there is a lot in line to be released in future which requires significant effort to complete. And there is no major contribution or maintainer who can save my time. So, either I can remove the post install script and limit my contribution to my spare time only which is not good for this project. Or I keep post-install script to let the users know about other opensource projects (not for funding but credit and advertisement).

But I understand your concern and I'm happy to find alternatives. The one idea is to maintain the parallel commercial project without any funding message and post-install script so that the users can use this library as per their need. I would be happy to know if there is any idea in your mind.

@G-Rath
Copy link
Contributor Author

G-Rath commented Dec 15, 2020

which is not the right fit to "npm fund".

neither is postinstall, which is meant for running actions that are required as part of the installing of a package after the install step has been finished (i.e compiling C dependencies).

Because of this, packages with postinstall scripts are actually deoptimized by package managers as they can't be cached due to postinstall being an indicator of external behaviour.

My point is between the two, fund is closer to what you're wanting than postinstall, since it enables user discovery in an opt-in manner.

(in fact, it's technically a violation of the terms of service of the npm registry to use postinstall for advertising messages)

I believe it is just a single line in your logs.

That entirely depends on the package manager, verbosity settings, etc.

In my original comment on the issue, I gave an example:

❯ npm i @aws-sdk/client-s3

> fast-xml-parser@3.17.5 postinstall /project/node_modules/fast-xml-parser
> node tasks/postinstall.js || exit 0

Love fast-xml-parser? Check https://amitkumargupta.work for more projects and contribution.

That's 5 lines, which you can't stop - even if you had an env-based check that said "don't log message" like OpenCollective and co do, it'll still trigger the other 4 lines.

This is also assuming that it doesn't error, and you're also including color characters in the message, which most CI systems won't render, meaning my logs are being filled with broken hard to parse text.

You could add a package like chalk, or a more lightweight package like is-ci to detect if you're in CI and should use those control characters but then you're introducing a new set of dependencies just for your one line message.

I support you giving people a way to find out about other projects, but postinstall isn't meant to be used for this.

@amitguptagwl
Copy link
Member

Hmm. Let's remove it for the time being until I'm with some no solution situation.

@amitguptagwl amitguptagwl merged commit 8c0f61b into NaturalIntelligence:master Dec 16, 2020
@svrooij
Copy link

svrooij commented Dec 16, 2020

@G-Rath thank you for creating this PR

@amitguptagwl I can still recommend using the npm fund options for this. Strictly speaking, asking for exposure or code contributions is the same as asking for money (in my opinion). I created a Github Sponsor account. That is great to get started when you want to make some money for your code.

An other way is maybe to add a message to the readme of this library, like this:

If you like using this library, please add the following message to the readme of the library that uses fast-xml-parser.

This library uses **fast-xml-parser**, check-out other the other apps Amit build on his website: [url]

I'm more then happy to put something like that in the readme of the sonos library.

@G-Rath G-Rath deleted the remove-postinstall-script branch December 16, 2020 18:31
@G-Rath
Copy link
Contributor Author

G-Rath commented Mar 6, 2021

@amitguptagwl
Copy link
Member

I have updated the license information to make it straight and simple. I hope it can help.

@amitguptagwl
Copy link
Member

@G-Rath 3.19.0 is published with license information update.

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.

Remove post install?

4 participants