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

Try @guardian/source package once more #1435

Merged
merged 13 commits into from
May 13, 2024
Merged

Try @guardian/source package once more #1435

merged 13 commits into from
May 13, 2024

Conversation

sndrs
Copy link
Member

@sndrs sndrs commented May 9, 2024

What are you changing?

create a single @guardian/source package (again).

Warning

This PR does not get rid of the old ones yet. They should still be what people use. This version could go away again pretty quickly.

Why?

managing peerDeps between tightly bound packages is hard, especially when they live in he same repo.

ideally you want the code on the disk to be the code that runs i.e. if you change source-foundations you want to see that reflected in source-react-components while developing.

but that means keeping the peerDep range of source-react-components in sync with the current version of source-foundations.

This in turn means releasing a major version of source-react-components everytime there's a new version of source-foundations, even if it's a patch.

This is the correct thing to do, but it's not desirable.

We concluded that was a smell coming from the package set up. The Source packages are tightly coupled, and the reason they are separate now is because we didn't understand properly how to support the old Node module resolution algorithm with sub-packages. We also didn't know about peerDependenciesMeta last time we looked at it.

Using those, we can create a single Source package that maps:

  • @guardian/source-foundations@guardian/source/foundations
  • @guardian/source-react-components@guardian/source/react-components

We can also make React/emotion optional peerDeps for people not using the react components.

Most package managers will install peerdeps anyway, so the optional flag just ensures that any version a consumer may also install matches the one we need.


Co-authored with @oliverabrahams

Copy link

changeset-bot bot commented May 9, 2024

🦋 Changeset detected

Latest commit: e215f5b

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@sndrs sndrs added the 🐥 Canaries Triggers canary releases of any packages with changesets waiting. label May 9, 2024
Copy link
Contributor

github-actions bot commented May 9, 2024

Tip

Once this PR is ready to go, add the run_chromatic label to run the Chromatic tests.

This saves us a lot of money by not running the tests before we need them.

@github-actions github-actions bot added the 📦 npm Affects a @guardian package on NPM label May 9, 2024
Copy link
Contributor

github-actions bot commented May 9, 2024

Note

The following canaries were published to NPM:

🐥

Copy link
Contributor

github-actions bot commented May 9, 2024

Note

The following canaries were published to NPM:

🐥

Copy link
Contributor

github-actions bot commented May 9, 2024

Note

The following canaries were published to NPM:

🐥

Copy link
Contributor

github-actions bot commented May 9, 2024

Note

The following canaries were published to NPM:

🐥

Copy link
Contributor

github-actions bot commented May 9, 2024

Note

The following canaries were published to NPM:

🐥

@sndrs sndrs removed the 🐥 Canaries Triggers canary releases of any packages with changesets waiting. label May 9, 2024
Copy link
Contributor

github-actions bot commented May 9, 2024

Note

The following canaries were published to NPM:

🐥

@oliverabrahams oliverabrahams added the run_chromatic Runs chromatic when label is applied label May 10, 2024
@sndrs sndrs changed the title try single source package again Try @guardian/source package once more May 13, 2024
@sndrs sndrs marked this pull request as ready for review May 13, 2024 11:06
@sndrs sndrs requested review from a team as code owners May 13, 2024 11:06
sndrs and others added 8 commits May 13, 2024 12:35
Co-authored-by: Oliver Abrahams <ollie.abrahams@guardian.co.uk>
Co-authored-by: Oliver Abrahams <ollie.abrahams@guardian.co.uk>
Co-authored-by: Oliver Abrahams <ollie.abrahams@guardian.co.uk>
sndrs and others added 5 commits May 13, 2024 12:35
Co-authored-by: Alex Sanders <alex@sndrs.dev>
Co-authored-by: Alex Sanders <alex@sndrs.dev>
Co-authored-by: Alex Sanders <alex@sndrs.dev>
@sndrs sndrs enabled auto-merge (squash) May 13, 2024 11:45
Copy link
Contributor

@jamesmockett jamesmockett left a comment

Choose a reason for hiding this comment

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

📦📦→📦 🚀

@sndrs sndrs merged commit e60f79e into main May 13, 2024
20 checks passed
@sndrs sndrs deleted the oa/sndrs/single-source branch May 13, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 npm Affects a @guardian package on NPM run_chromatic Runs chromatic when label is applied
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants