Skip to content

Allow passing a custom loader such as from yoga-wasm-web - #699

Closed
pavi2410 wants to merge 1 commit into
vercel:mainfrom
pavi2410:custom-yoga-loader
Closed

Allow passing a custom loader such as from yoga-wasm-web#699
pavi2410 wants to merge 1 commit into
vercel:mainfrom
pavi2410:custom-yoga-loader

Conversation

@pavi2410

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings August 15, 2025 15:54
@pavi2410
pavi2410 requested a review from shuding as a code owner August 15, 2025 15:54
@vercel

vercel Bot commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
satori-playground Ready Ready Preview Comment Aug 15, 2025 3:54pm

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modifies the Yoga layout engine initialization to allow for custom loaders, enabling flexibility for different Yoga implementations like yoga-wasm-web.

  • Replaces automatic Yoga preloading with lazy initialization
  • Adds optional yogaLoader parameter to getYoga() function with default fallback
  • Caches the yoga promise to prevent multiple initializations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Comment thread src/utils.ts
let yogaPromise: Promise<Yoga>
export function getYoga(yogaLoader: () => Promise<Yoga> = loadYoga) {
if (!yogaPromise) {
yogaPromise = yogaLoader()

Copilot AI Aug 15, 2025

Copy link

Choose a reason for hiding this comment

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

The API design could lead to inconsistent behavior. If different callers pass different yogaLoader functions, only the first one will be used due to the caching mechanism. Consider either preventing this scenario or documenting this behavior clearly.

Suggested change
yogaPromise = yogaLoader()
let yogaPromise: Promise<Yoga>
let yogaLoaderUsed: (() => Promise<Yoga>) | undefined
/**
* Returns a promise for the Yoga instance.
* Only the first loader function passed will be used; subsequent calls with a different loader will throw an error.
*/
export function getYoga(yogaLoader: () => Promise<Yoga> = loadYoga) {
if (!yogaPromise) {
yogaPromise = yogaLoader()
yogaLoaderUsed = yogaLoader
} else if (yogaLoaderUsed && yogaLoaderUsed !== yogaLoader) {
throw new Error(
'getYoga: yogaLoader function can only be set once. Subsequent calls must use the same loader.'
)

Copilot uses AI. Check for mistakes.
Comment thread src/utils.ts
export function getYoga(yogaLoader: () => Promise<Yoga> = loadYoga) {
if (!yogaPromise) {
yogaPromise = yogaLoader()
}

Copilot AI Aug 15, 2025

Copy link

Choose a reason for hiding this comment

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

This implementation has a potential race condition. If multiple calls to getYoga() happen simultaneously before yogaPromise is set, multiple yoga instances could be created. Consider using a more robust initialization pattern or synchronization mechanism.

Suggested change
}
// Use nullish coalescing assignment to ensure only one promise is created
yogaPromise ??= yogaLoader()

Copilot uses AI. Check for mistakes.
@shuding

shuding commented Aug 28, 2025

Copy link
Copy Markdown
Member

Covered by #705

@shuding shuding closed this Aug 28, 2025
@pavi2410
pavi2410 deleted the custom-yoga-loader branch August 30, 2025 10:27
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