Skip to content

feat: add pnpm workspace for submodule local linking#157

Draft
garrity-miepub wants to merge 1 commit intomainfrom
feat/pnpm-workspace-submodules
Draft

feat: add pnpm workspace for submodule local linking#157
garrity-miepub wants to merge 1 commit intomainfrom
feat/pnpm-workspace-submodules

Conversation

@garrity-miepub
Copy link
Copy Markdown
Contributor

Configure pnpm workspace so submodules under packages/ automatically resolve @mieweb/ui to the local source instead of npm.

  • Add pnpm-workspace.yaml declaring packages/* as workspace members
  • Add .npmrc with link-workspace-packages=true (pnpm v10 default is false)
  • Add pnpm.overrides in package.json to force @mieweb/ui -> link:. (needed to break cyclic dependency: root -> datavis -> root)
  • Add packages/README.md documenting the setup and how to add new submodules

When a submodule is cloned standalone, the same package.json works unchanged — it fetches @mieweb/ui from npm as usual.

Configure pnpm workspace so submodules under packages/ automatically
resolve @mieweb/ui to the local source instead of npm.

- Add pnpm-workspace.yaml declaring packages/* as workspace members
- Add .npmrc with link-workspace-packages=true (pnpm v10 default is false)
- Add pnpm.overrides in package.json to force @mieweb/ui -> link:.
  (needed to break cyclic dependency: root -> datavis -> root)
- Add packages/README.md documenting the setup and how to add new submodules

When a submodule is cloned standalone, the same package.json works
unchanged — it fetches @mieweb/ui from npm as usual.
Copilot AI review requested due to automatic review settings April 1, 2026 21:25
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR configures the repository as a pnpm workspace so git submodules under packages/ can resolve @mieweb/ui to the local workspace source during development, while still allowing standalone submodule clones to install from npm.

Changes:

  • Add pnpm-workspace.yaml to include packages/* as workspace members.
  • Add .npmrc to enable workspace package linking.
  • Add pnpm.overrides to force @mieweb/ui to resolve to link:., and document the workflow in packages/README.md.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pnpm-workspace.yaml Declares packages/* as pnpm workspace members.
.npmrc Enables workspace linking behavior for pnpm.
package.json Adds pnpm.overrides to force local linking of @mieweb/ui.
packages/README.md Documents how workspace-based submodule linking works and how to add new submodules.
pnpm-lock.yaml Updates lockfile for the workspace + overrides and introduces a packages/datavis importer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

`@mieweb/ui` to resolve as `link:.` for all workspace members regardless of
cycles. No additional configuration is needed.

### 4. (Optional) Submodule uses npm internally
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The section numbering is duplicated: both this heading and the previous one are labeled "### 4". Renumber this heading to keep the step sequence consistent.

Suggested change
### 4. (Optional) Submodule uses npm internally
### 5. (Optional) Submodule uses npm internally

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +87
In the submodule's own `package.json`, list `@mieweb/ui` with a normal semver
range (not `workspace:`):

```json
{
"dependencies": {
"@mieweb/ui": "^0.2.2"
}
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

This doc states submodules should depend on @mieweb/ui via a normal semver range (e.g. ^0.2.2) so the same package.json works when cloned standalone. However, the generated pnpm-lock.yaml in this PR shows packages/datavis currently has @mieweb/ui with specifier: link:../.., which would not work outside the workspace. Either update the documentation to reflect the current state (and any exceptions), or ensure the submodule dependency is expressed as a semver range so standalone clones behave as described.

Copilot uses AI. Check for mistakes.
Comment on lines +178 to +179
specifier: link:../..
version: link:../..
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

packages/datavis is recorded as depending on @mieweb/ui via specifier: link:../... If the goal is for submodules to be clonable/instalable standalone (falling back to the npm-published @mieweb/ui), this link: specifier will prevent that. Consider changing the submodule's dependency to a semver range and relying on the workspace override/linking to resolve it locally when inside this repo.

Suggested change
specifier: link:../..
version: link:../..
specifier: ^0.0.0
version: 0.0.0

Copilot uses AI. Check for mistakes.
Comment on lines +2289 to +2292
eslint@10.1.0:
resolution: {integrity: sha512-S9jlY/ELKEUwwQnqWDO+f+m6sercqOPSqXM5Go94l7DOmxHVDgmSFGWEzeE/gwgTAr0W103BWt0QLe/7mabIvA==}
engines: {node: ^20.19.0 || ^22.13.0 || >=24}
hasBin: true
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The workspace lock now includes eslint@10.1.0 (pulled in via the packages/datavis workspace importer), which declares a Node engine of ^20.19.0 || ^22.13.0 || >=24. This effectively raises the minimum Node version needed for a full workspace install above the repo's declared engines.node (>=18). If Node 18 is still intended to be supported for contributors/CI, consider aligning the submodule toolchain versions with Node 18, adjusting the repo's Node engine/CI version, or otherwise preventing Node>=20-only devDependencies from being installed as part of the root workspace install.

Copilot uses AI. Check for mistakes.
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