-
-
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
add out option to adapter-node, and add types for adapters #531
Conversation
@@ -1,3 +1,5 @@ | |||
import { Logger } from './types.internal'; |
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.
It seems like Logger
isn't really an internal type if it's being exposed here. Why not just put it in this file?
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.
it's internal insofar as you're not going to do
/** @type {import('@sveltejs/kit').Logger} */
— it's always just a property of the builder
(which also needn't be exported) that's passed to the adapt
function. this file should only export types that need to be consumed by users of Kit
* out?: string; | ||
* }} options | ||
*/ | ||
export default function ({ out = 'build' } = {}) { |
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.
should we have an interface for this?
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.
nah, it's adapter-specific
@@ -1,8 +1,8 @@ | |||
/** | |||
* @param {import('../../types').Request} request | |||
* @param {import('../../../types.internal').Request} request |
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 would expect that Request
and Response
should not be internal because almost every adapter will have to deal with them
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.
probably. we can move things out of internal as and when necessary
Tests are failing but I'm pretty confident it's entirely due to flakiness rather than anything in this PR, so I'll merge this |
Makes the output directory for
adapter-node
configurable. While I was at it, I added anAdapter
type, but haven't yet applied it to the other adapters.This entailed moving some types out of
kit/src/types.d.ts
intokit/types.internal.d.ts
so that they can more easily get included in the package, which makes the PR look much bigger than it is.