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: Prevent Yoga init from running more than once #2878

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jmezzacappa
Copy link

@jmezzacappa jmezzacappa commented Sep 23, 2024

When quickly calling loadYoga() multiple times (in a script or web server that generates PDFs, for example), Yoga would try to initialize multiple times and eventually throw a strange error like:

BindingError: Expected null or instance of Config, got an instance of Config

Fixes #2892

Copy link

changeset-bot bot commented Sep 23, 2024

🦋 Changeset detected

Latest commit: b25e27f

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 Patch
@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

When quickly calling `loadYoga()` multiple times (in a script that
generates PDFs, for example), Yoga would try to initialize multiple
times and eventually throw a strange error like:

`Expected null or instance of Config, got an instance of Config`
@jmezzacappa
Copy link
Author

jmezzacappa commented Sep 23, 2024

@diegomura I just noticed "drop cjs support" in the changelog. I could change this to use a top-level await, if preferred. Edit: On second thought, that could slow down app startup times in many cases

@gitaugakwa
Copy link

Any update on this?

@holvold
Copy link

holvold commented Sep 27, 2024

I have the same issue! Sometimes the iFrame remains blank after rendering and I get BindingError: Expected null or instance of Config, got an instance of Config in the console.

@jgo80
Copy link

jgo80 commented Oct 1, 2024

I run into this issue when creating multiple PDFs at the same time with Promise.all

@gitaugakwa
Copy link

@jgo80 Issue is also what I run into. I haven't run @jmezzacappa changes yet but looking at it, it should be alright and quite straightforward to merge. @diegomura could we get a merge?

@HuguesBlanco HuguesBlanco mentioned this pull request Oct 2, 2024
@jmezzacappa
Copy link
Author

jmezzacappa commented Oct 3, 2024

Just in case anyone might find this helpful:

In my case, we are rendering PDFs in a web server. As a workaround until this PR gets merged/released, I implemented a queue to prevent the server from trying to render multiple PDFs at once.

Here's the code
import { renderToStream } from '@react-pdf/renderer'
import LinkedList from 'mnemonist/linked-list'
import stream from 'node:stream'
import { createElement } from 'react'
import { PDFDoc, type PDFDocProps } from './components'

interface PDFTaskCtx {
	pdfProps: PDFDocProps

	resolvers: {
		resolve: (stream: stream.Readable) => void
		reject: (reason: unknown) => void
	}

	signal: AbortSignal
}

async function renderTask({
	pdfProps,
	resolvers,
	signal,
}: PDFTaskCtx): Promise<void> {
	try {
		signal.throwIfAborted()

		const element = createElement(PDFDoc, pdfProps)
		const pdfStream = await renderToStream(element).then(readableStream =>
			// react-pdf's stream is typed as a NodeJS.ReadableStream interface, which
			// may or may not be a stream.Readable instance.
			readableStream instanceof stream.Readable
				? stream.addAbortSignal(signal, readableStream)
				: stream.Readable.from(readableStream, { objectMode: false, signal }),
		)

		resolvers.resolve(pdfStream)

		// Wait for the stream to finish before moving on to the next PDF
		await stream.promises.finished(pdfStream)
	} catch (error) {
		resolvers.reject(error)
	}
}

/** This ensures that only one PDF is rendered at a time */
const enqueueTask = /* @__PURE__ */ (() => {
	// An array could work here instead, if you don't expect a lot of tasks to be
	// queued up at once. This linked list implementation just provides better
	// performance, as both `.push()` and `.shift()` are O(1) operations. For
	// small queues, this would make no human-perceivable difference.
	const pendingTasks = new LinkedList<() => Promise<void>>()

	async function noop() {}

	let runTasks = async () => {
		// Prevent running multiple times, simultaneously
		const runTasksImpl = runTasks
		runTasks = noop

		try {
			while (true) {
				const task = pendingTasks.shift()
				if (task === undefined) break
				await task()
			}
		} finally {
			runTasks = runTasksImpl
		}
	}

	return function enqueueTask(task: () => Promise<void>) {
		pendingTasks.push(task)
		runTasks()
	}
})()

export function renderPdf(
	pdfProps: PDFDocProps,
	signal: AbortSignal,
): Promise<stream.Readable> {
	// Note: Promise.withResolvers is not supported in Node.js versions below v22,
	// but it's easy to polyfill/ponyfill.
	// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/withResolvers
	const resolvers = Promise.withResolvers<stream.Readable>()
	enqueueTask(() => renderTask({ pdfProps, resolvers, signal }))
	return resolvers.promise
}

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.

BindingError
4 participants