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: Pin supported target environment #1170

Merged
merged 2 commits into from Sep 19, 2022

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Sep 19, 2022

What:

Closes #1169

Why:

A minor release created a build with that contained syntax that we don't support yet.

How:

Run npx browserslist defaults with a version of caniuse-lite that was used at the time of the latest good release.
That snapshot is now contained in our manifest and ensures build tools transpile to this target.
Having "defaults" as the target environment is too weak of a contract to have.
Now it's apparent that changing the target environment is considered a breaking change.
We can decide in the next major release what a good matrix for supported runtimes is.

Checklist:

  • [ ] Documentation added to the
    docs site
  • Tests rm -rf node_modules/; npm install; npm run build, check dist/@testing-library/dom.esm.js, no more ??
  • [ ] TypeScript definitions updated
  • Ready to be merged

Copy link
Member

@MichaelDeBoey MichaelDeBoey left a comment

Choose a reason for hiding this comment

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

@eps1lon Pinning kcd-scripts won't help, as we're not using latest version yet (as I described in #1169 (comment))

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 19, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a06325d:

Sandbox Source
react-testing-library-examples Configuration

@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #1170 (a06325d) into main (ab8182c) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main     #1170   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          996       996           
  Branches       327       327           
=========================================
  Hits           996       996           
Flag Coverage Δ
node-12 100.00% <ø> (ø)
node-14 100.00% <ø> (ø)
node-16 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@eps1lon
Copy link
Member Author

eps1lon commented Sep 19, 2022

Pinning kcd-scripts won't help, as we're not using latest

Please see the PR title and description. I'm not pinning kcd-scripts but everything.

@eps1lon eps1lon changed the title fix: Pin build dependencies fix: Pin supported target environment Sep 19, 2022
"safari 15.5",
"samsung 17.0",
"samsung 16.0",
"node 12.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

kcd-scripts does not use the engines field in its "--bundle" target. This is incorrect since it conflates the module entry with the browser entry. module and main are only for the used module system. Their runtime target needs to be the same. This isn't formally specified but conventionally used by bundlers (see https://esbuild.github.io/api/#main-fields).

Copy link
Member

Choose a reason for hiding this comment

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

@eps1lon eps1lon marked this pull request as ready for review September 19, 2022 19:39
@eps1lon eps1lon merged commit a9a8cf2 into testing-library:main Sep 19, 2022
@eps1lon eps1lon deleted the fix/pin-tooling branch September 19, 2022 19:40
@github-actions
Copy link

🎉 This PR is included in version 8.18.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@MatanBobi
Copy link
Member

MatanBobi commented Jun 16, 2023

@eps1lon, it looks like op_mob 64 isn't available as our CI failed for that in this PR and we should upgrade this to 73 but I'm not sure if that will add the nullish coalescing issue again.

CC: @timdeschryver

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.

Nullish coalescing operator (??) in dist in 8.18.0
3 participants