Skip to content

Conversation

@rotu
Copy link
Contributor

@rotu rotu commented Jan 6, 2025

This allows this package to be used as a dependency (in built form) even when the compilation tools can't be installed.

This allows this package to be used as a dependency (in built form) even when the compilation tools can't be installed.
@aminya
Copy link
Member

aminya commented Mar 24, 2025

With the changes in #712, I don't think we need this anymore. The recent cmake-ts changes remove a lot of dependencies and bundle them as binary to be backwards compatible with older Node versions regardless of its dependencies.

@aminya aminya closed this Mar 24, 2025
@rotu
Copy link
Contributor Author

rotu commented Mar 24, 2025

Yay! Glad to see this get reconciled with mainline cmake-ts! I'm still hazy on why cmake-ts is in dependencies and not optionalDependencies or devDependencies; it seems to only be required if npm_config_build_from_source is true.

@aminya
Copy link
Member

aminya commented Mar 24, 2025

It will be needed if the prebuilds are not available, or they fail to load for that specific platform.

@rotu
Copy link
Contributor Author

rotu commented Mar 24, 2025

It will be needed if the prebuilds are not available, or they fail to load for that specific platform.

I'm building for Docker, where I intend to specify the platform at the time the package is installed. cmake-ts and node-addon-api are not needed at runtime, so it would be nice to strip them (and their dependencies) from the image. The image doesn't have cmake and other build dependencies anyway, so building won't succeed even if the native binary is absent.

@aminya
Copy link
Member

aminya commented Mar 24, 2025

That's a valid use case, but not sure if we can support it for the expense of builds not working for some users. You can patch the dependencies of zeromq via the following methods:

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.

2 participants