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

Change config prerender.force to prerender.onError #2007

Merged

Conversation

happycollision
Copy link
Contributor

@happycollision happycollision commented Jul 25, 2021

fixes #1940

Essentially, we are doing this:

kit: {
  prerender: {
    crawl: boolean;
    enabled: boolean;
-   force: boolean;
+   onError: 'fail' | 'continue' | ((details: RequestDetails) => void | never);
    pages: string[];
  }
}
  • Changing from force to onError gives a somewhat more descriptive
    name to the option.
  • Using the strings "fail" and "continue" further clarifies what the
    option does.
  • Providing your own function to deal with an error allows for
    fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.

Teaching the new option via error message:

This PR also adds a failing option check for using the old force property, and gives you the proper value to use in its place as well. This should help when people version bump to the latest build.

CleanShot 2021-07-24 at 20 53 16@2x

CleanShot 2021-07-24 at 22 43 13@2x

Examples of various acceptable options, and the output when a link is broken

The new handler function option

Throw to fail

CleanShot 2021-07-26 at 20 53 51@2x

Anything but throwing will continue the build

CleanShot 2021-07-26 at 21 00 43@2x

The old behavior via the new settings

fail

CleanShot 2021-07-24 at 20 59 05@2x

continue

CleanShot 2021-07-24 at 20 58 22@2x

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Jul 25, 2021

🦋 Changeset detected

Latest commit: c6f5a10

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit 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

@happycollision happycollision force-pushed the change-prerender-force-to-onError branch 9 times, most recently from eacff40 to a49af69 Compare July 25, 2021 03:25
@happycollision happycollision force-pushed the change-prerender-force-to-onError branch 2 times, most recently from 3599432 to d90b018 Compare July 26, 2021 13:55
@happycollision
Copy link
Contributor Author

All set. Thanks @benmccann!

@benmccann
Copy link
Member

Lgtm. I left a handful of minor comments. I really like the idea of using a function to give the user more control and stole the idea for another PR #2008

@happycollision
Copy link
Contributor Author

@benmccann , in this final pass, I took a little more liberty with the names of those properties (verb became referenceType) and also with the call sites for the error function (as I mentioned I would in my response to your request about types.

Additionally, on the types front, I created local references for types that are used more than once (or are especially long) near the top of the file. They can be inlined if that is more desirable, but I feel that things are more readable this way.

fixes sveltejs#1940

- Changing from `force` to `onError` gives a somewhat more descriptive
  name to the option.
- Using the strings "fail" and "continue" further clarifies what the
  option does.
- Providing your own function to deal with an error allows for
  fine-tuning which errors fail the build and which do not.

The original ticket suggested that the custom function return a boolean,
but after seeing the implementation of the error handler in svelteKit, I
thought it more fitting to just allow it the exact same API: throw if
you want the build to fail.
This commit can be reverted for the 1.0 release. It is purely here for
ease of transition from `force` to `onError`.
@happycollision happycollision force-pushed the change-prerender-force-to-onError branch from a31d066 to c6f5a10 Compare July 27, 2021 03:47
@benmccann benmccann merged commit 20dad18 into sveltejs:master Jul 27, 2021
sidharthv96 added a commit to sidharthv96/kit that referenced this pull request Jul 31, 2021
…ePath

* 'master' of https://github.com/sveltejs/kit: (85 commits)
  [chore] minor simplification of `parse_body` (sveltejs#2043)
  Version Packages (next) (sveltejs#2026)
  [chore] deduplicate config types (sveltejs#2030)
  [chore] remove invalid css declaration (sveltejs#2038)
  [fix] correctly pass Vite options in preview mode (sveltejs#2036)
  [feat] Ignore adapter build files (sveltejs#1924)
  [chore] Upgrade Rollup
  Don't check external links on prerender (sveltejs#1679)
  Version Packages (next) (sveltejs#2017)
  [chore] convert remaining type aliases (sveltejs#2018)
  [feat] explicitly set compilerOptions.hydratable to config.kit.hydrate (sveltejs#2024)
  [feat] More powerful and configurable rendering options (sveltejs#2008)
  [chore] typecheck example (sveltejs#2019)
  Change config `prerender.force` to `prerender.onError` (sveltejs#2007)
  [chore] prefer interfaces to types (sveltejs#2010)
  [docs] minor wording improvement in migration guide (sveltejs#2006)
  [chore] test debugging improvements
  [docs] fix typo (sveltejs#2003)
  [chore] use @ts-expect-error instead of @ts-ignore (sveltejs#1999)
  Version Packages (next) (sveltejs#1996)
  ...
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.

Configurable error handling for prerender
2 participants