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

fix: bump @adobe/css-tools for ESM support #525

Merged
merged 1 commit into from Aug 24, 2023

Conversation

jgoz
Copy link
Collaborator

@jgoz jgoz commented Aug 23, 2023

What:

Fixes #524.

Why:

#519 added ESM support, but previous versions of @adobe/css-tools were not ESM compatible.

How:

Changed package.json

Checklist:

  • Documentation
  • Tests
  • Updated Type Definitions
  • Ready to be merged

@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Merging #525 (2353eaa) into main (853a3e5) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #525   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines          664       664           
  Branches       251       251           
=========================================
  Hits           664       664           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jgoz
Copy link
Collaborator Author

jgoz commented Aug 23, 2023

@nickmccurdy

Copy link
Member

@nickmccurdy nickmccurdy left a comment

Choose a reason for hiding this comment

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

Blocking this on either the addition of automated tests or a reproduction of the original issue so I can test it manually.

Copy link
Contributor

@wojtekmaj wojtekmaj left a comment

Choose a reason for hiding this comment

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

Can confirm this fixes the issue.

@nickmccurdy It's going to be hard to automate this, because the problem is in the historical lockfile entries. You'll not encounter this problem when doing a fresh install without a lockfile. So you would need to "simulate the past" to do this. If you really want to however, here's how to do it (I'm providing instructions for Yarn 3 because that's what I use, but it'd be similar for any package manager):

  • Install @testing-library/jest-dom & vitest (or Jest, but Vitest is natively ESM so it's faster to reproduce the issue)
  • Find the following entry in the lockfile:
    "@adobe/css-tools@npm:^4.0.1":
      version: 4.3.1
      resolution: "@adobe/css-tools@npm:4.3.1"
      checksum: ad43456379ff391132aff687ece190cb23ea69395e23c9b96690eeabe2468da89a4aaf266e4f8b6eaab53db3d1064107ce0f63c3a974e864f4a04affc768da3f
      languageName: node
      linkType: hard
    Note that ^4.0.1 resolved to 4.3.1 now, but it must have resolved to an earlier version in the past. So to simulate this…
  • Replace with:
    "@adobe/css-tools@npm:^4.0.1":
      version: 4.2.0
      resolution: "@adobe/css-tools@npm:4.2.0"
      checksum: dc5cc92ba3d562e7ffddb79d6d222c7e00b65f255fd2725b3d71490ff268844be322f917415d8c4ab39eca646343b632058db8bd5b1d646193fcc94d1d3e420b
      languageName: node
      linkType: hard
  • Install again
  • Observe the crash

@frontendphil
Copy link

I've seen the same issue in our codebase (even though I also cannot share a reproduction case). However, when I added a pnpm override to use @adobe/css-tools@4.3.0, the particular error disappeared, but I got TypeError: Cannot convert undefined or null to object instead. So, the version bump might not be the ideal fix for this issue.

@indooorsman
Copy link

in my project @adobe/css-tools was resolved and locked to 4.0.1 in past, and it does not have esm exports, as a result all my testings failed. uninstall it then reinstall it should work.

Copy link
Member

@nickmccurdy nickmccurdy left a comment

Choose a reason for hiding this comment

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

I can now confirm this is broken when using @jgoz's reproduction and installing @testing-library/jest-dom, but fixed when linking the built package with this PR. Thanks!

@nickmccurdy nickmccurdy changed the title fix: Bump @adobe/css-tools fix: bump @adobe/css-tools for ESM support Aug 24, 2023
@nickmccurdy nickmccurdy merged commit b959a68 into testing-library:main Aug 24, 2023
7 checks passed
@github-actions
Copy link

🎉 This PR is included in version 6.1.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6.1.0 @adobe/css-tools crashing
5 participants