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

Swap out yarn for pnpm #795

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

daniel-heppner-ibigroup
Copy link
Contributor

Yarn 1 has been unsupported for a while, with the last update 3 years ago. The biggest problem with running such an old version is that we are missing out on the more ergonomic ways of referencing internal dependencies.

Instead of needing to stay on top of internal dependency versions, we can now just reference workspace:* as our version number for any internal dependency.

Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

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

Can we have it auto-prepublish again? Also can we make sure that published packages refer to actual package versions and not the workspace things

@daniel-heppner-ibigroup
Copy link
Contributor Author

From the docs:
"Before publish, aliases are converted to regular aliased dependencies. The above example will become: "bar": "npm:[email protected]"."
https://pnpm.io/workspaces

@daniel-heppner-ibigroup
Copy link
Contributor Author

prepublish now runs after install automatically. Since internal dependencies are symlinked instead of downloaded from npm, this is necessary for things to work properly. I think both of your comments are now addressed.

@daniel-heppner-ibigroup
Copy link
Contributor Author

It looks like the SVGs are changing in the snapshots because the version of styled icons changed.

image I'm seeing slight visual differences on them but nothing substantial and not related to the build system.

@miles-grant-ibigroup
Copy link
Collaborator

Things are looking much better. Is there any way we can update the yarn prepublish to add a warning about using pnpm instead?

BREAKING CHANGE: maplibre-gl is no longer a direct dependency
Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

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

Do we still need update-internal-dependencies.js?

@@ -24,18 +24,7 @@ See:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Quite a few missing reference changes in here, especially around workspace package version management

@miles-grant-ibigroup
Copy link
Collaborator

Seeing this error when trying to import in otp-rr: error Couldn't find any versions for "@opentripplanner/map-popup" that matches "workspace:*"\n +`

pnpm build is not working?

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