Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

Check that we are exporting valid CSS #1566

Merged
merged 9 commits into from
Oct 4, 2022
Merged

Conversation

ob6160
Copy link
Contributor

@ob6160 ob6160 commented Oct 3, 2022

What is the purpose of this change?

In some cases, Emotion style CSS is exported from source-foundations. Applications that consume these exported styles can have an expectation that only CSS is provided, but right now we make no guarantee that this is the case.

This breaks the build in projects like dotcom rendering (AMP specifically), because these invalid CSS rules are being rendered directly onto the page, without going through the Emotion stack.

This change fixes some invalid CSS comments that use // vs /* .. */ in our reset which could affect the cascade. We also add unit tests using the lightningcss project to parse our exported CSS so we have confidence that we are shipping valid code.

What does this change?

  • Adds a custom "Jest matcher" called .toBeValidCSS() so we can make assertions in a natural way against the CSS that we export using the lightningcss library.
  • Adds these assertions to our unit tests for the CSS resets and accessibility classes that we export.

This PR was split out from #1554 so we could make it distinct from other changes to export emotion styles from the typography module.

@ob6160 ob6160 requested a review from a team as a code owner October 3, 2022 17:22
@changeset-bot
Copy link

changeset-bot bot commented Oct 3, 2022

🦋 Changeset detected

Latest commit: f469890

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

This PR includes changesets to release 1 package
Name Type
@guardian/source-foundations Patch

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

@github-actions github-actions bot added the foundations Affects @guardian/source-foundations label Oct 3, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

yarn.lock changes

Click to toggle table visibility
Name Status Previous Current
detect-libc ADDED - 1.0.3
lightningcss-darwin-arm64 ADDED - 1.16.0
lightningcss-darwin-x64 ADDED - 1.16.0
lightningcss-linux-arm-gnueabihf ADDED - 1.16.0
lightningcss-linux-arm64-gnu ADDED - 1.16.0
lightningcss-linux-arm64-musl ADDED - 1.16.0
lightningcss-linux-x64-gnu ADDED - 1.16.0
lightningcss-linux-x64-musl ADDED - 1.16.0
lightningcss-win32-x64-msvc ADDED - 1.16.0
lightningcss ADDED - 1.16.0

Copy link
Contributor

@joecowton1 joecowton1 left a comment

Choose a reason for hiding this comment

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

Much clearer! Thanks for breaking it out.

@ob6160 ob6160 merged commit eeecc02 into main Oct 4, 2022
@ob6160 ob6160 deleted the ob/lightningcss-assertions branch October 4, 2022 09:40
@ob6160
Copy link
Contributor Author

ob6160 commented Oct 6, 2022

@sndrs sndrs linked an issue Oct 7, 2022 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
foundations Affects @guardian/source-foundations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Untangling emotion from source-foundations
2 participants