Skip to content

Allow post install script#31

Merged
robin-drexler merged 1 commit into
mainfrom
allow-post-install-script
May 22, 2025
Merged

Allow post install script#31
robin-drexler merged 1 commit into
mainfrom
allow-post-install-script

Conversation

@robin-drexler
Copy link
Copy Markdown
Member

@robin-drexler robin-drexler commented May 20, 2025

Snapit does a checkout of the code instead of running in the existing action context, which makes it hard to make changes to files before snapit runs.

build_script exists, but it runs after snapit has already ran changeset version which might be too late in certain cases.

This PR introduces a post_install_script optional input param that allows to pass a script that runs right after dependencies have been installed.

In our specific instance we want to run changeset pre exit to allow snaphots for a package that is currently in pre mode.
More on this: https://changesets-docs.vercel.app/en/prereleases

You can't version snapshot packages when pre mode is on.
We do have pre mode on for the next RC in https://github.com/Shopify/ui-extensions/ because it allows us to release versions like 2025-10.rc-1 and add ~2025-10.rc-0 to extension templates as a workaround to not being able to put a tag there in npm: npm/cli#8252

We might want to have snapit do changeset pre exit by itself at some point, but I think it's too early to add this.
We're still in the process of figuring out how pre mode works exactly and I think having a more generic post install hook is a sensible addition regardless of our current use case.

🎩

Tested successfully in the ui-extensions repo:
Shopify/ui-extensions#2901 (comment)
https://github.com/Shopify/ui-extensions/actions/runs/15144950812

Comment thread .github/dependabot.yaml
updates:
- package-ecosystem: github-actions
directory: "/"
directory: '/'
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

prettier did this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No problems with this!

@robin-drexler robin-drexler requested a review from vividviolet May 20, 2025 16:21
@robin-drexler robin-drexler force-pushed the allow-post-install-script branch from 5a4b755 to def30cd Compare May 20, 2025 16:26
@robin-drexler robin-drexler marked this pull request as ready for review May 20, 2025 17:16
Copy link
Copy Markdown
Member

@vividviolet vividviolet left a comment

Choose a reason for hiding this comment

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

👍 This is a good addition to snapit

@robin-drexler robin-drexler requested a review from BPScott May 20, 2025 19:03
@alex-page
Copy link
Copy Markdown
Member

@robin-drexler why not just include it in your package as a postinstall script? Unsure why snapit needs to have this as a feature. Is there a time when you don't want this postinstall script to run?

Seems like it's just adding an extra place for functions to run that is unnecessary.

@alex-page alex-page requested a review from aaronccasanova May 20, 2025 23:37
@robin-drexler
Copy link
Copy Markdown
Member Author

@robin-drexler why not just include it in your package as a postinstall script? Unsure why snapit needs to have this as a feature. Is there a time when you don't want this postinstall script to run?

Seems like it's just adding an extra place for functions to run that is unnecessary.

We don't want to run this every time after dependencies are installed. Only when running in the snapit gh action because you can't create a snapshot version if you're in pre mode so we're temporarily disabling it in the snapit action.

Copy link
Copy Markdown
Member

@alex-page alex-page left a comment

Choose a reason for hiding this comment

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

I want to be careful about adding multiple lifecycle hooks and focus on keeping snapit extremely simple. This makes sense though, thanks @robin-drexler

@robin-drexler robin-drexler merged commit ec24b7a into main May 22, 2025
2 checks passed
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.

4 participants