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: disable postinstall script prior to publishing #925

Closed
wants to merge 1 commit into from

Conversation

brandonlenz
Copy link
Contributor

@brandonlenz brandonlenz commented Feb 19, 2021

Note!!:

The alternative to this is to roll back husky to v4: #926. We should merge one and close the other + delete the branch.

Due to Husky v5 currently causing more headaches than it may be worth, I recommend rolling back to v4 (PR linked above)

Summary

Husky 5's implementation is a bit different and required adding a postinstall script to package.json. This script should only be ran while developing, not for users of the packaged artifact.

Solution is to disable post install scripts before publishing. This is what pinst was added for in #867, however I missed adding the disable. (See pinst usage for more info)

closes: #923

How To Test

  1. Checkout this branch:
git fetch
git checkout bl-fix-husky-packaged-post-install-923
  1. Perform a clean install to verify dev still works (I deleted node_modules and ran yarn to verify, plus the ability to commit this and see the git hooks still working with husky were good tests)

  2. Locally running yarn prepublishOnly should run the existing test and linting scripts as well as disabling postInstall:
    image
    image

  3. locally running yarn postpublish should result in postInstall being enabled once again:
    image
    image

  4. Finally, testing an install of this package after release of this code change will be the true test.

@@ -1 +1 @@
_
_
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the original PR I added a newline to this .gitignore because it's generally good practice for files to end with newlines. However, every time husky install runs (which happens after every yarn install, it updates this file. Might as well merge it in this PR so it won't show up as a random diff for anyone.

Copy link
Contributor Author

@brandonlenz brandonlenz Feb 19, 2021

Choose a reason for hiding this comment

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

For reference since github doesn't really show this change very well on the Conversation tab:
image

@trussworks-infra-zz
Copy link

Warnings
⚠️

Changes were made to package.json, but not to yarn.lock - Perhaps you need to run yarn install?

Generated by 🚫 dangerJS against 09aa811

@brandonlenz brandonlenz marked this pull request as ready for review February 19, 2021 20:24
@brandonlenz brandonlenz changed the title fix: disable postinstall script for non-dev (packaged) fix: disable postinstall script prior to publishing Feb 19, 2021
@brandonlenz
Copy link
Contributor Author

Closing as we are going with #926 for the near-term at least.

@brandonlenz brandonlenz deleted the bl-fix-husky-packaged-post-install-923 branch February 19, 2021 22:38
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.

[fix] Latest release (1.12.0) cannot be installed
2 participants