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

Add a document about pnpm monorepo error #1349

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

mansona
Copy link
Member

@mansona mansona commented Feb 1, 2023

I desperately need some help explaining this better. I only have a rough understanding of what the underlying mechanism is so I am not feeling confident explaining it

@ef4
Copy link
Contributor

ef4 commented Feb 1, 2023

Thanks for working on this. Yeah, I think the text needs to be reworked. It's not really a document about pnpm, although it can mention pnpm as one of the reasons that a package may not really get consumed by broccoli and thus end up empty in embroider.

An attempt (I don't think I did a good job yet explaining the pnpm part):

Problem: Embroider Produces an Empty Addon Package

Embroider rewrites traditional (v1) addons to the new v2 format before building your application. It can only tell what contents those packages should have by letting the traditional broccoli-based build pipelines run and observing the files that come out.

But there are several reasons an addon won't ever get consumed by broccoli, causing embroider to emit an empty package, whose package.json has a comment linking back to this document.

Reasons you might have an empty package include:

  • if an app uses addons.blacklist, the excluded addon can end up empty since nobody consumed it
  • if an addon uses the shouldIncludeChildren() hook to exclude a child addon, the child addon can end up empty
  • if you have multiple copies of certain addons (@ember/test-helpers and @ember/test-waiters are known examples) within your node_modules tree, one copy may end up unconsumed, ending up empty
  • if you use pnpm, you may not think you have multiple copies of an addon but still end up with multiple copies because pnpm is more strictly correct about peer dependencies. A library that should see two different peers when consumed from two different places will appear as two distinct copies to consumers.

@mansona mansona force-pushed the pnpm-bug-docs branch 2 times, most recently from 21c3e7d to bac94ec Compare February 2, 2023 11:05
@mansona
Copy link
Member Author

mansona commented Feb 2, 2023

I took a second pass at it there now mostly taking your stuff to replace the majority of the content 😂 I also renamed the file to make it more about the empty package output rather than a pnpm bug 👍


There are several reasons an addon won't ever get consumed by broccoli, causing embroider to emit an empty package. Some of those reasons include:

- if an app uses `addons.blacklist`, the excluded addon can end up empty since nobody consumed it
Copy link
Contributor

@bertdeblock bertdeblock Feb 3, 2023

Choose a reason for hiding this comment

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

Suggested change
- if an app uses `addons.blacklist`, the excluded addon can end up empty since nobody consumed it
- if an app uses `addons.blacklist` or `addons.exclude`, the excluded addon can end up empty since nobody consumed it

Though, addons.blacklist will be removed in ember-cli v5.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've included this change 👍

There are several reasons an addon won't ever get consumed by broccoli, causing embroider to emit an empty package. Some of those reasons include:

- if an app uses `addons.blacklist`, the excluded addon can end up empty since nobody consumed it
- if an addon uses the shouldIncludeChildren() hook to exclude a child addon, the child addon can end up empty
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- if an addon uses the shouldIncludeChildren() hook to exclude a child addon, the child addon can end up empty
- if an addon uses the `shouldIncludeChildAddon()` hook to exclude a child addon, the child addon can end up empty

Copy link
Member Author

Choose a reason for hiding this comment

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

I've included this change 👍

@simonihmig
Copy link
Collaborator

if you use pnpm, you may not think you have multiple copies of an addon but still end up with multiple copies because pnpm is more strictly correct about peer dependencies. A library that should see two different peers when consumed from two different places will appear as two distinct copies to consumers.

For visibility, it seems this is getting fixed here: pnpm/pnpm#6153 (opt-in for v7, probably by default in v8)

@ef4
Copy link
Contributor

ef4 commented Jun 5, 2023

Thanks, merging this so we can move forward with a link to this doc, and then we can iterate on it further.

@ef4 ef4 merged commit 22e656d into embroider-build:main Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants