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

feat!: update Yoga to 3.0 #2711

Merged
merged 4 commits into from
Sep 22, 2024
Merged

feat!: update Yoga to 3.0 #2711

merged 4 commits into from
Sep 22, 2024

Conversation

wojtekmaj
Copy link
Contributor

Closes #2692
Closes #2507

This PR updates Yoga to 3.0.

See the list of new features, improvements, and potential breaking changes in #2692.

Copy link

changeset-bot bot commented Apr 10, 2024

🦋 Changeset detected

Latest commit: d21bd48

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

This PR includes changesets to release 5 packages
Name Type
@react-pdf/layout Major
@react-pdf/renderer Patch
@react-pdf/examples Patch
@react-pdf/e2e-node-cjs Patch
@react-pdf/e2e-node-esm 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

Closes #2692
Closes #2507

This PR updates Yoga to 3.0.

See the list of new features, improvements, and potential breaking changes in #2692.
@diegomura
Copy link
Owner

@wojtekmaj this looks good. What's missing?

@wojtekmaj
Copy link
Contributor Author

@diegomura See the original ticket: we will be unable to provide CJS builds as Yoga 3 is a pure ESM module.

@diegomura
Copy link
Owner

Sorry! Missed that. Huh, good decision to make. Would be nice to push forward that and just embrace ESM but I'm afraid that would break a lot of setups. Will think about this. Any input would be welcomed!

@wojtekmaj
Copy link
Contributor Author

wojtekmaj commented Apr 23, 2024

I think the question is more WHEN and not IF.

It's really up to you: how much further are you able (and willing to!) push the v2 forward? If you think that significant improvements can still be made, you may want to hold off with v3 for a while, to avoid maintaining two releases at the same time. If you think v2 is "pretty much ready" and you're basically only maintaining it, then it would be best to bite the bullet and remove CJS builds.

The move will also improve the experience for ESM users (see #2507), userbase of which is ever growing, at the expense of CJS users, userbase of which is ever shrinking.

Please note that removing CJS builds does not stop CJS users from:

  • Using v2 🤷 They clearly like outdated solutions anyway
  • Importing ESM modules into CJS asynchronously (although that's a bit of a burden)
  • Using Node with --experimental-require-module flag which may just work
  • Using a bundler like Rollup before running code in Node to benefit from their built-in ESM/CJS interop to avoid all module resolution issues

@karlhorky
Copy link
Contributor

I wonder if Yoga v2 with its unusual CommonJS wrapper code is also causing issues like this property access error Yoga.Overflow.Hidden that we've been experiencing:

@karlhorky
Copy link
Contributor

karlhorky commented Jun 13, 2024

@wojtekmaj is there an easy way to try this PR out?

If this isn't published somewhere already, would it be possible to get a prerelease version published?

I'm currently applying your changes to @react-pdf/layout in node_modules by hand using patch-package and using pnpm Overrides to force [email protected], and it appears to be working, but it's a bit brittle...

@wojtekmaj
Copy link
Contributor Author

I think this is the best we can do until it's released :(

@karlhorky
Copy link
Contributor

Oh ok, @diegomura would you be open to a pre-release version like @react-pdf/[email protected] being published?

@wojtekmaj aside from that, would you be open to publish a scoped package under your name eg. @wojtekmaj/react-pdf-layout ? I think that could be used in package.json with the npm: prefixes... 🤔

@wojtekmaj wojtekmaj marked this pull request as ready for review June 19, 2024 10:22
@michaelgmcd
Copy link

It may actually be more beneficial to go back to keeping things async and using the yoga-layout/load export. The default one requires top-level await (may break some users):
https://github.com/facebook/yoga/tree/main/javascript#requirements

@wojtekmaj
Copy link
Contributor Author

Thanks for the heads up. PDF.js introduced top-level await recently and they rolled it back after a huge backslash, so I guess you'd be right.

@seanroddy-fbg
Copy link

Hi, is there an expected release data for this feature?

@diegomura
Copy link
Owner

diegomura commented Sep 22, 2024

I'm very tempted on removing CJS support. We should look forward. I feel it's kind of our responsibility to help things moving forward as lib authors. Newer solutions won't become adopted if people do not have an incentive to upgrade. Also, given bandwidth of lib authors feels like it simplifies things. Many packages here won't even need a build step. @wojtekmaj curious what you think

@wojtekmaj
Copy link
Contributor Author

I can't agree more. And frankly, it's not even a question if we want to move react-pdf forward. More and more libs are abandoning CJS, like our core dependencies.

@diegomura
Copy link
Owner

Amazing. I'll remove CJS support and we can land this after then!

@diegomura
Copy link
Owner

Done in #2871

Copy link
Owner

@diegomura diegomura left a comment

Choose a reason for hiding this comment

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

utACK

@diegomura diegomura merged commit 4257032 into diegomura:master Sep 22, 2024
@wojtekmaj wojtekmaj deleted the yoga-3 branch September 23, 2024 07:43
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.

Update Yoga to 3.0 [solution] __dirname is not defined in ES module scope
5 participants