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

npm init -- --app should create an "install" hook #1042

Closed
dherman opened this issue May 16, 2024 · 8 comments
Closed

npm init -- --app should create an "install" hook #1042

dherman opened this issue May 16, 2024 · 8 comments

Comments

@dherman
Copy link
Collaborator

dherman commented May 16, 2024

There's no "install" hook for --app projects created by the new create-neon.

@kjvalencik
Copy link
Member

Should it be prepare and exist for both apps and libraries?

@dherman
Copy link
Collaborator Author

dherman commented May 19, 2024

Should it be prepare and exist for both apps and libraries?

Libraries aren't built on demand, they're built remotely on the CI/CD server across the full platform matrix and the binaries are published separately. The library itself doesn't require any local build process.

For apps, I think you're right that "prepare" is the right call.

dherman added a commit that referenced this issue May 19, 2024
@dherman
Copy link
Collaborator Author

dherman commented May 19, 2024

I added this to #1041.

@kjvalencik
Copy link
Member

The prepare hook only runs before a publish. That's not something we want to take advantage of for libs?

@dherman
Copy link
Collaborator Author

dherman commented May 19, 2024

The prepare hook runs both before publish and install. You might be thinking of prepublishOnly, which only runs before publish.

For libs, we can't make use of a publish hook because it's synchronous, and libs need to fork off a bunch of remote prebuilds running on CI/CD servers. So publishing is actually done via CI/CD instead of locally. But if you can think of a way to do this with an npm publish workflow, I'm open to it!

@kjvalencik
Copy link
Member

Is there some other hook we can use that always gets run when you locally pull for both types?

@dherman
Copy link
Collaborator Author

dherman commented May 20, 2024

Can you say more? I don't quite know what you mean by locally pull.

@dherman
Copy link
Collaborator Author

dherman commented May 20, 2024

Followup: based on discussion in #1041, we agreed that this isn't a great idea after all. Rationale:

  • There's no npm workflow for "please run this script the first time I set up node_modules".
  • It's not great to force a rebuild every time you update a dependency.
  • We don't really know how apps will set up their builds, so it's best not to try to guess, and just leave it to them.
  • Consistency between both app and lib: if you want to run a local build, do npm run build.

@dherman dherman closed this as completed May 20, 2024
dherman added a commit that referenced this issue May 20, 2024
- small cleanups to `inferOrg` regexp logic
- revert the `"prepare"` hook for --app (see #1042 (comment) )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants