Skip to content

fix: add Streamable.from to avoid eager promises#2143

Merged
migueloller merged 2 commits into
canaryfrom
miguel/eng-7914-apply-ppr-and-dynamicio-patterns-from-pdp-to-rest-of-app
Mar 25, 2025
Merged

fix: add Streamable.from to avoid eager promises#2143
migueloller merged 2 commits into
canaryfrom
miguel/eng-7914-apply-ppr-and-dynamicio-patterns-from-pdp-to-rest-of-app

Conversation

@migueloller

@migueloller migueloller commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

What/Why?

Our Streamable pattern relies on passing promises as props to RSCs and client components. For RSCs, it "pushes down" data fetching so that Next.js will mark particular components as dynamic when using PPR. For client components, it streams the data to the client while triggering a Suspense boundary.

For the former case, we were getting the following error:

Unhandled Rejection: Error: Route /[locale]/category/[slug] needs to bail out of prerendering at this point because it used `await searchParams`, `searchParams.then`, or similar. React throws this special object to indicate where. It should not be caught by your own try/catch. Learn more: https://nextjs.org/docs/messages/ppr-caught-error
    at P (.next/server/chunks/5884.js:1:42808)
    at Object.get (.next/server/chunks/4019.js:4:32956)
    at <unknown> (.next/server/app/[locale]/(default)/(faceted)/category/[slug]/page.js:42:741) {
  ‘$$typeof’: Symbol(react.postpone)
}
Node.js process exited with exit status: 128. The logs above can help with debugging the issue.

We believe the reason for this is that promises are eager, not lazy. This means that if we call an async function on an RSC, it will start executing even if the RSC that we pass the promise to hasn't rendered yet. In particular, if the execution calls cookies(), await searchParams or other dynamic APIs which throw, it results in the error above.

This PR introduces Streamable.from which abstracts the idea of a "lazy promise", deferring execution until the first .then call. It uses Streamable.from anywhere we were calling an async function without await to avoid the unhandled rejection.

Something particular about Streamable.from is that it allows us to refer to values via closure directly in the RSC, removing the need for creating async functions when using the streamable pattern. This PR purposefully keeps this out of scope and I plan to address this in a future PR.

Furthermore, the implementation of Streamable.from is a simple wrapper around p-lazy. In a future PR, I intend to remove the dependency on p-lazy and have a custom implementation for lazy promises.

This PR is pre-work for the move to PPR + dynamicIO to better optimize caching and performance of Catalyst.

Testing

CleanShot.2025-03-24.at.17.52.10.mp4

@migueloller
migueloller requested a review from a team March 24, 2025 21:51
@linear

linear Bot commented Mar 24, 2025

Copy link
Copy Markdown

@changeset-bot

changeset-bot Bot commented Mar 24, 2025

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b8732362cee4e340ed998cba3c38b760ded0a181

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@vercel

vercel Bot commented Mar 24, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
catalyst-canary ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 25, 2025 2:37pm
4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
catalyst ⬜️ Ignored (Inspect) Mar 25, 2025 2:37pm
catalyst-au ⬜️ Ignored (Inspect) Visit Preview Mar 25, 2025 2:37pm
catalyst-soul ⬜️ Ignored (Inspect) Visit Preview Mar 25, 2025 2:37pm
catalyst-uk ⬜️ Ignored (Inspect) Visit Preview Mar 25, 2025 2:37pm

@fikrikarim fikrikarim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great!

Comment on lines 364 to 367

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we automatically wrap every Streamable.all promise with Streamable.from?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found myself wondering if we needed Streamable.all if we had Streamable.from and I think the answer is "yes" given that they have different goals. Streamable.all will combine multiple streamables and keep that stable so that we don't get infinite suspensions in the client side. There's a question of whether we want to be doing that at all, though. I'm wondering if we should remove Streamable.all. Because we shouldn't be creating new streamables in client components, just how we shouldn't be creating promises in client components and because on the server we can use Streamable.from

That being said, you might have a point here about Streamable.all potentially eagerly executing both of these promises. I'll look into it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, so I tested it and Streamable.all does trigger the executor, which is problematic.

Also, I remembered that the reason we added Streamable.all was because we wanted to be able to use VIBES components that were "shared" components with Makeswift, which would execute in a client environment.

I'm going to have to re-think Streamable.all but I will do this in a separate PR. cc: @agurtovoy

@migueloller
migueloller force-pushed the miguel/eng-7914-apply-ppr-and-dynamicio-patterns-from-pdp-to-rest-of-app branch from b873236 to fad33a3 Compare March 25, 2025 14:36
@migueloller
migueloller added this pull request to the merge queue Mar 25, 2025
Merged via the queue into canary with commit f16a6be Mar 25, 2025
@migueloller
migueloller deleted the miguel/eng-7914-apply-ppr-and-dynamicio-patterns-from-pdp-to-rest-of-app branch March 25, 2025 14:52
@github-actions

Copy link
Copy Markdown
Contributor

⚡️🏠 Lighthouse report

Lighthouse ran against https://catalyst-canary-mx1yj5yh7-bigcommerce-platform.vercel.app

🖥️ Desktop

We ran Lighthouse against the changes on a desktop and produced this report. Here's the summary:

Category Score
🟢 Performance 99
🟢 Accessibility 92
🟠 Best practices 78
🟠 SEO 82

📱 Mobile

We ran Lighthouse against the changes on a mobile and produced this report. Here's the summary:

Category Score
🟠 Performance 84
🟢 Accessibility 92
🟠 Best practices 78
🟠 SEO 85

migueloller added a commit that referenced this pull request Mar 31, 2025
* fix: switching locales redirects user to the home page (#1990)

* chore: add demo section to readme (#1996)

* Update translations (#1993)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>

* feat: manage locales when creating storefronts from the CLI (#1986)

* feat: manage locales

* Add changeset

---------

Co-authored-by: Nathan Booker <nathan.booker@bigcommerce.com>

* feat: add default Tailwind typography styles (#2007)

* Update translations (#2004)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>

* fix(core): handle registration errors in form (#2006)

* chore(release): publish create-catalyst CLI (#2015)

* chore: remove scoped version bump

* chore(release): publish create-catalyst CLI

* feat: add es-419 to list of allowed locale for selecting (#2017)

* feat(cli): scaffold package (#2010)

* fix(core): show coupon discounts as a separate summary item (#2084)

* fix(core): show coupon discounts as a separate summary item

* fix: typo in changeset

* feat(core): add shipping options to checkout (#1997)

* feat(core): add shipping options to checkout

* fix: capitalization of Edit

* fix: bump vibes

* fix: update vibes

* fix: remove else and early return

* refactor: simplify logic by using a Set for countries

* fix: hide forms visually

* fix: add no shipping options message

* fix: remove reduce and use Set, remove let variable

* fix: split mutations

* fix: disable complexity rule for Cart

* Update translations (#2104)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>

* refactor: update signin behavior to use different providers (#2103)

* refactor: update signin behavior to use different providers

* chore: add additional folders to lint task

* feat: Sync Button and DynamicForm with VIBES (#2110)

* feat(core): add compare functionality to PLP (#2060)

* feat(core): migrate compare page to use soul

* feat: hide compare checkbox dynamically

* feat: pass in comparable products

* fix: add optimistic items

* feat: add optimistic to remove

* feat: add optimistic to checkbox

* fix: sort when added

* fix: sort groups and use link to compare

* fix: bump vibes components, fix button

* fix: remove unused primitves breadcrumbs component

* fix: sync vibes components

* fix: wrap transitions and call optimistic before setParam

* fix: validate with zod variables are correct

* fix: expose remove label

* fix: await setParam

* fix: translate remove label:

* fix: use parser and loader for compare

* feat: alert when max compare limit is set, disable checkboxes

* feat: Sync ProductCard and WishlistItemCard with VIBES (#2112)

* feat: Sync Breadcrumbs (#2117)

* feat: Add Wishlist VIBES components (#2118)

* chore(security): add documentation around CSRF checks (#2114)

* fix: set the trustHost value with an env var

* chore(core): documentation around CSRF checks within next-auth

* feat: Add wishlist translations, transformers, and core components (#2119)

* fix(core): revert to ids search param in compare page (#2125)

* feat(client): add errorPolicy option to GQL requests (#2124)

* feat: add anonymous session with cart tracking (#2111)

* refactor(core): abstract cart creation and addition logic (#2126)

* feat: Add wishlist modals, actions, and visibility switch (#2120)

* Bump next to address CVE (#2132)

* feat: Add wishlist modal defs, share button, and action dropdown menu (#2121)

* chore: update pnpm to 10.6.5 (#2135)

* Update translations (#2133)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
Co-authored-by: Jorge Moya <jorge.moya@bigcommerce.com>

* chore: update node to version 22 (#2136)

* chore: remove invalid changeset (#2139)

* feat: Add wishlist pages (#2122)

* fix(core): ignore errors in orders and render products without links (#2141)

* fix: add `Streamable.from` to avoid eager promises (#2143)

* feat: add Streamable.from

* fix: use Streamable.from instead of eager promises

* chore: update dependencies to latest minors (#2142)

* feat: group 1 update for Soul form components (#2130)

* feat(core): add store hash meta tag (#2147)

* fix: dedupe default image in PDP (#2162)

* feat: group 2 update for Soul form components (#2160)

* fix: remove unused components (#2163)

---------

Co-authored-by: Aleksey Gurtovoy <agurtovoy@acm.org>
Co-authored-by: Chancellor Clark <chancellor.clark@bigcommerce.com>
Co-authored-by: bc-svc-local <102379007+bc-svc-local@users.noreply.github.com>
Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
Co-authored-by: Roman Krasinskyi <49246595+RomanKrasinskyi@users.noreply.github.com>
Co-authored-by: Nathan Booker <nathan.booker@bigcommerce.com>
Co-authored-by: Hunter Garrett <hunter@hunterbecton.com>
Co-authored-by: Jorge Moya <jorge.moya@bigcommerce.com>
Co-authored-by: Daniel Almaguer <daniel.almaguer@bigcommerce.com>
Co-authored-by: Jordan Arldt <jordan.arldt@bigcommerce.com>
Co-authored-by: Nathan Booker <bookernath@users.noreply.github.com>
chanceaclark added a commit that referenced this pull request Apr 18, 2025
* fix: switching locales redirects user to the home page (#1990)

* chore: add demo section to readme (#1996)

* Update translations (#1993)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>

* feat: manage locales when creating storefronts from the CLI (#1986)

* feat: manage locales

* Add changeset

---------

Co-authored-by: Nathan Booker <nathan.booker@bigcommerce.com>

* feat: add default Tailwind typography styles (#2007)

* Update translations (#2004)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>

* fix(core): handle registration errors in form (#2006)

* chore(release): publish create-catalyst CLI (#2015)

* chore: remove scoped version bump

* chore(release): publish create-catalyst CLI

* feat: add es-419 to list of allowed locale for selecting (#2017)

* feat(cli): scaffold package (#2010)

* fix(core): show coupon discounts as a separate summary item (#2084)

* fix(core): show coupon discounts as a separate summary item

* fix: typo in changeset

* feat(core): add shipping options to checkout (#1997)

* feat(core): add shipping options to checkout

* fix: capitalization of Edit

* fix: bump vibes

* fix: update vibes

* fix: remove else and early return

* refactor: simplify logic by using a Set for countries

* fix: hide forms visually

* fix: add no shipping options message

* fix: remove reduce and use Set, remove let variable

* fix: split mutations

* fix: disable complexity rule for Cart

* Update translations (#2104)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>

* refactor: update signin behavior to use different providers (#2103)

* refactor: update signin behavior to use different providers

* chore: add additional folders to lint task

* feat: Sync Button and DynamicForm with VIBES (#2110)

* feat(core): add compare functionality to PLP (#2060)

* feat(core): migrate compare page to use soul

* feat: hide compare checkbox dynamically

* feat: pass in comparable products

* fix: add optimistic items

* feat: add optimistic to remove

* feat: add optimistic to checkbox

* fix: sort when added

* fix: sort groups and use link to compare

* fix: bump vibes components, fix button

* fix: remove unused primitves breadcrumbs component

* fix: sync vibes components

* fix: wrap transitions and call optimistic before setParam

* fix: validate with zod variables are correct

* fix: expose remove label

* fix: await setParam

* fix: translate remove label:

* fix: use parser and loader for compare

* feat: alert when max compare limit is set, disable checkboxes

* feat: Sync ProductCard and WishlistItemCard with VIBES (#2112)

* feat: Sync Breadcrumbs (#2117)

* feat: Add Wishlist VIBES components (#2118)

* chore(security): add documentation around CSRF checks (#2114)

* fix: set the trustHost value with an env var

* chore(core): documentation around CSRF checks within next-auth

* feat: Add wishlist translations, transformers, and core components (#2119)

* fix(core): revert to ids search param in compare page (#2125)

* feat(client): add errorPolicy option to GQL requests (#2124)

* feat: add anonymous session with cart tracking (#2111)

* refactor(core): abstract cart creation and addition logic (#2126)

* feat: Add wishlist modals, actions, and visibility switch (#2120)

* Bump next to address CVE (#2132)

* feat: Add wishlist modal defs, share button, and action dropdown menu (#2121)

* chore: update pnpm to 10.6.5 (#2135)

* Update translations (#2133)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
Co-authored-by: Jorge Moya <jorge.moya@bigcommerce.com>

* chore: update node to version 22 (#2136)

* chore: remove invalid changeset (#2139)

* feat: Add wishlist pages (#2122)

* fix(core): ignore errors in orders and render products without links (#2141)

* fix: add `Streamable.from` to avoid eager promises (#2143)

* feat: add Streamable.from

* fix: use Streamable.from instead of eager promises

* chore: update dependencies to latest minors (#2142)

* feat: group 1 update for Soul form components (#2130)

* feat(core): add store hash meta tag (#2147)

* fix: dedupe default image in PDP (#2162)

* feat: group 2 update for Soul form components (#2160)

* fix: remove unused components (#2163)

---------

Co-authored-by: Aleksey Gurtovoy <agurtovoy@acm.org>
Co-authored-by: Chancellor Clark <chancellor.clark@bigcommerce.com>
Co-authored-by: bc-svc-local <102379007+bc-svc-local@users.noreply.github.com>
Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
Co-authored-by: Roman Krasinskyi <49246595+RomanKrasinskyi@users.noreply.github.com>
Co-authored-by: Nathan Booker <nathan.booker@bigcommerce.com>
Co-authored-by: Hunter Garrett <hunter@hunterbecton.com>
Co-authored-by: Jorge Moya <jorge.moya@bigcommerce.com>
Co-authored-by: Daniel Almaguer <daniel.almaguer@bigcommerce.com>
Co-authored-by: Jordan Arldt <jordan.arldt@bigcommerce.com>
Co-authored-by: Nathan Booker <bookernath@users.noreply.github.com>
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.

3 participants