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

Propose pnpm support #907

Merged
merged 3 commits into from
Jun 2, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
167 changes: 167 additions & 0 deletions text/0907-pnpm-support.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
---
stage: accepted
start-date: 2023-03-06T14:09:00.000Z
release-date: # In format YYYY-MM-DDT00:00:00.000Z
release-versions:
teams: # delete teams that aren't relevant
- cli
- learning
prs:
accepted: https://github.com/emberjs/rfcs/pull/907
project-link:
suite:
---

<!---
Directions for above:

stage: Leave as is
start-date: Fill in with today's date, 2032-12-01T00:00:00.000Z
release-date: Leave as is
release-versions: Leave as is
teams: Include only the [team(s)](README.md#relevant-teams) for which this RFC applies
prs:
accepted: Fill this in with the URL for the Proposal RFC PR
project-link: Leave as is
suite: Leave as is
-->

# `pnpm` support

## Summary

Enable Ember CLI users to opt into using `pnpm` for package management.

## Motivation

[`pnpm`](https://pnpm.io/) is a popular alternative to both `npm` and `yarn` that prioritizes correctness, especially for `peerDependencies` and monorepos.

`pnpm` has very active maintainance, support, and financial funding.
Their [website](https://pnpm.io/) states that `pnpm` is:
- Fast: `pnpm` is up to 2x faster than the alternatives
- Efficient: Files inside `node_modules` are cloned or hard linked from a single content-addressable storage
- Supports monorepos: `pnpm` has built-in support for multiple packages in a repository
- Strict: `pnpm` creates a non-flat `node_modules` by default, so code has no access to arbitrary packages.

For folks with lots of projects on their computers, `pnpm` is _extremely_ space-efficient.
Where `npm` and `yarn` would duplicate `node_modules` per-project, `pnpm` will only download a package (at a specific version) once across your whole machine.

Additionally, `npm` and `yarn` repeatedly have demonstrated that their strategies with `peerDependencies` are not correct, and it is vitally important we use and support a tool that can guide folks towards correctly creating addons. For example, `@embroider/macros` relies on node's resolution algorithm, so having `peerDependencies` resolved correctly is important for `dependencySatisfies` to work as expected in monorepos.

Ember CLI currently only supports `npm` (default) and `yarn` for project initialization as well as various commands. At present, projects work with `pnpm`, but the tooling is unaware.



## Detailed design

Enabling `pnpm` is designed as opt-in to prevent disruptions to developer's current workflow, much the same as `yarn`.

There are a few integration points that we must support for `pnpm` (and any package manager):
- `ember install`
- `ember init`, `ember new`, `ember addon`
- `ember try:one`, `ember try:each`
- generated C.I. configs
- Documenation

### `ember install`

There are two mechanisms through which to opt-in.
The first one is the presence of a `pnpm-lock.yaml` file in the project root.

The `pnpm-lock.yaml` file is generated by `pnpm` when you run `pnpm install` (or the shorter `pnpm i`),
so we assume that its presence means the developer intends to use `pnpm` to manage their dependencies.

Alternatively you, you can force Ember CLI to use `pnpm` with the `--pnpm` flag, and symmetrically,
you can force Ember CLI to not use `pnpm` with the `--no-pnpm` flag.

To recap:

- `ember install ember-resources` with `pnpm-lock.yaml` present will use `pnpm`
- `ember install ember-resources` without `pnpm-lock.yaml` present will use npm
- `ember install ember-resources --pnpm` will use `pnpm`
- `ember install ember-resources --no-pnpm` will use npm
Copy link
Member

Choose a reason for hiding this comment

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

What's the use case for this? If you had a pnpm-lock.yaml and didn't want to use pnpm?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

It's a consequence of our arg-parsing tool. boolean flags have equiv negated no-prefixed versions


### `ember init`, `ember new`, `ember addon`

Since this triad of commands is generally ran before a project is set up, there is no `pnpm-lock.yaml` file presence to check.
This means we are left with the `--pnpm`/`--no-pnpm` pair of flags, that will also be added to these commands:

- `ember new my-app` will use npm
- `ember new my-app --pnpm` will use `pnpm`

The above also applies to `ember addon` and `ember init`, noting that `ember init` doesn't receive any arguments.

#### `--skip-npm`

The `--skip-npm` flag _actually means_ "skip installation of dependencies" when using `ember addon` and `ember new`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we propose a change of this flag to --skip-deps? Maybe that's separate but I can see it being related.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

I think we should -- I personally don't think that needs an RFC, (and is out of scope for this one) but I think it should happen.
RFC could then be put up for deprecating --skip-npm

including skipping installation with both `npm` and `yarn`.
This will need to be extended to also skip installation of dependencies when `pnpm` is used.

For example:
```bash
ember new my-app --pnpm --skip-npm
```


### `ember try:one`, `ember try:each`

At the time of writing this RFC, `ember-try` already supports `pnpm`, but it is undocumented in [the README](https://github.com/ember-cli/ember-try).
Documentation will need to be added to the README,
as well as the relevant `ember-cli` blueprints will need to correctly configure `usePnpm: true` in the `ember-try.js` config file when the `pnpm` flag is present.


### generated C.I. configs

At the time of writing this RFC, `ember-cli` supports two C.I. environments: Travis and GitHub Actions.

Both the `.travis.yml` and `.github/workflows/ci.yml` config files for relevant blueprints will need to support the `pnpm` option such that C.I. passes on new projects using `pnpm`.

### for addons

Addons should be stricter than defaults, so that they are good stewards of the ecosystem and don't leak accidental uncertainty to their consumers.

In addon projects, the `.npmrc` will need the following defaults:
```
# all peer dependencies must be declared or forwarded to the consumer
auto-install-peers=false

# we want true isolation in addons -- if a dependency is not declared, we want an error
reslove-peers-from-workspace-root=false
```

### Documentation

These pages presently mention `npm` / `yarn` and will need to be updated to include `pnpm`
- https://cli.emberjs.com/release/basic-use/assets-and-dependencies/
- https://guides.emberjs.com/release/addons-and-dependencies/

## How we teach this

The Ember Guides should be updated to reflect the new flags, where applicable,
as well as the new behavior of `ember install` in the presence of a `pnpm-lock.yaml` -- though most of the guides use `ember` as the CLI tool for managing packages.

We may want to consider updating the [tutorial](https://guides.emberjs.com/release/tutorial/part-1/orientation/) (and its automation) to use `pnpm`, as `npm` is very slow.


In addition, the built-in instructions for `ember help` should be updated to reflect the new flags.


## Drawbacks

- `pnpm` is very strict about peers and what dependencies are allowed in a project and will error if a project's package.json is incorrect for a given scenario. `pnpm` is very clear about these errors and what to do for action items, but it means that we'll need to make sure that `pnpm` is tested in `ember-cli` when generating new projects so that we can be certain that folks' "first time experience" is smooth
- There may be other package managers in the future, but we cannot see the future. There have been talks about making ember-cli somehow generically handle package-managers, but it is unknown how that would work, and is unneeded for now.


## Alternatives

- Figure out a way to generically handle any package manager
- Continue with the partial pnpm support we have today

## Unresolved questions

- Are there any other references in the guides to `npm` or `yarn`?
The only place I could find that _mentioned_ `yarn` is here: https://guides.emberjs.com/release/addons-and-dependencies/
- Is there a `--package-manager` flag / option in `ember-cli`? for blueprint authors, that may be useful.
- Should `--skip-npm` be aliased to `--skip-install`?
Copy link
Contributor

@MelSumner MelSumner Mar 10, 2023

Choose a reason for hiding this comment

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

Ahh okay I see this part. Yes I think it should be aliased to something more generic. --skip-deps, --skip-install-deps, etc.