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

Introduce throwOnFailure option for prerender #956

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

resolritter
Copy link

@resolritter resolritter commented May 3, 2023

Problem

Due to some unknown issue with lazy, the prerender function was not working as intended for my components. I eventually overcame that problem, but I did not know it was was happening in the first plac until I looked into the generated HTML and noticed it was wrong. In other words, the prerender is failing silently since the error path is not being handled correctly, as I'll explain below.

The problem is related to this line:

let html = await render();

The render function was returning undefined because my components could not be rendered correctly. Then we go into the next line:

html += `<script type="isodata"></script>`;

Since html was undefined, the generated HTML would become undefined<script type="isodata"></script>, which is not what I want.

Solution

There should be a way of generating an exception in case the components cannot be rendered correctly. For that, this PR introduces the throwOnFailure option which controls what happens in case rendering fails. It defaults to a falsy value for preserving prerender's current behavior, but arguably it should be true.

@changeset-bot
Copy link

changeset-bot bot commented May 3, 2023

🦋 Changeset detected

Latest commit: d4f8dd9

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

This PR includes changesets to release 1 package
Name Type
preact-iso Minor

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

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.

1 participant