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

Configurational dependencies #8

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Configurational dependencies #8

wants to merge 6 commits into from

Conversation

zkochan
Copy link
Member

@zkochan zkochan commented Dec 26, 2024

Implementation: pnpm/pnpm#8915

Comment on lines +48 to +49
## Prior Art

Choose a reason for hiding this comment

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

Is this similar to build-dependencies in Rust?

Copy link
Member

Choose a reason for hiding this comment

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

I believe this is different than build-dependencies in the Rust ecosystem.

This RFC is similar what templates was trying to do with reusable configuration, but only for pnpm specific settings. Similar to templates, configuration is stored in the package manifest since that's available at earlier stages of the installation process. However, I think it's a lot simpler than templates and likely to be less controversial since it's only for pnpm related settings and not existing package.json fields.

Copy link
Member

@gluxon gluxon left a comment

Choose a reason for hiding this comment

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

Thanks for writing this up! I think this will be useful to a lot of users.

text/0004-configurational-dependencies.md Outdated Show resolved Hide resolved
Comment on lines +48 to +49
## Prior Art

Copy link
Member

Choose a reason for hiding this comment

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

I believe this is different than build-dependencies in the Rust ecosystem.

This RFC is similar what templates was trying to do with reusable configuration, but only for pnpm specific settings. Similar to templates, configuration is stored in the package manifest since that's available at earlier stages of the installation process. However, I think it's a lot simpler than templates and likely to be less controversial since it's only for pnpm related settings and not existing package.json fields.

* They will not have lifecycle scripts.
* They will only be installable via exact versions.

These dependencies will be installed into a new directory (name to be decided), not into `node_modules`.
Copy link
Member

Choose a reason for hiding this comment

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

That's a good idea.

Copy link
Member Author

@zkochan zkochan Dec 26, 2024

Choose a reason for hiding this comment

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

Yeah, using node_modules could have advantages but would also require additional changes in the existing installation code. Unless, we use a dot directory inside node_modules, like node_modules/.pnpm-config. Or node_modules/.pnpm/config. This way users wouldn't have to add new entries to .gitignore.

Copy link
Member

Choose a reason for hiding this comment

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

I do like the idea of node_modules/.pnpm-config as an analogue to the existing node_modules/.pnpm dir. I feel like that makes it clearer that these are NPM packages, but live under a namespace isolated from the existing packages in node_modules/.pnpm.

@@ -49,6 +49,8 @@ These dependencies will be installed as early as possible in order to be able to

## Prior Art

There was a big RFC by Brandon Cheng about a much more powerful system using [templates](https://github.com/pnpm/rfcs/pull/3), which was rejected.
Copy link
Member

Choose a reason for hiding this comment

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

The new system here targeting just pnpm specific settings is a lot easier to understand. 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

TBH, I don't remember why exactly it was rejected.

@zkochan
Copy link
Member Author

zkochan commented Dec 28, 2024

I will work on an implementation.

@zkochan
Copy link
Member Author

zkochan commented Dec 28, 2024

Related PR: pnpm/pnpm#8915

I think with configurational dependencies we can achieve some of what we wanted to achieve with templates. We could have some config dependency that changes the package.json of the project. For instance, adds some dev dependencies. For this to work we just need to run the config dependency from a prepare script and re-read the package.json file after executing the prepare script.

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.

3 participants