-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Better error logging #1062
Better error logging #1062
Conversation
* @param {string} path | ||
* @param {string} parent | ||
*/ | ||
async function visit(path, parent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parent_path
might be a clearer name? Or could update the JSDoc to clarify that it's a path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like it's obvious from usage. If you go by the name alone then you might think that the parent_path
of /foo/bar
was /foo
, whereas if you scan down to where it's used, it becomes clear what it does
Cool!! I wonder if there are any tests we should add for this |
Prerendering tests in general are a bit of a TODO |
There's a regression when a unicode path is If it's changed, say, to: await visit(decodeURI(path), parsed.pathname.replace(config.kit.paths.base, '')); It builds correctly. |
@datio can you file a new issue instead of commenting on a closed PR? this will for sure get lost otherwise |
This started out as a PR to address #982 specifically, but I ended up fixing a bunch of adjacent code:
SSRRenderOptions
type is much neater, with the options passed toapp.render
separated outSSRRenderState
as distinct fromSSRRenderOptions
— this is used for tracking prerender dependencies and preventing infinite fetch loops. It means that the built app only needs to create the render options object onceBefore submitting the PR, please make sure you do the following
Tests
pnpm test
and lint the project withpnpm lint
Changesets
pnpx changeset
and following the prompts