Add types for vue-server-renderer (fix #5772) #5775
Conversation
|
|
||
| export declare function createRenderer(options?: RendererOptions): Renderer; | ||
|
|
||
| export declare function createBundleRenderer(bundle: any, options?: BundleRendererOptions): BundleRenderer; |
There was a problem hiding this comment.
http://ssr.vuejs.org/en/api.html#createbundlerendererbundle-options
IMHO a bundle is either a string or an object.
| interface RendererOptions { | ||
| template?: string; | ||
| inject?: boolean; | ||
| shouldPreload?: (file: string, type: string) => boolean; |
There was a problem hiding this comment.
We can even refine type to a precise list of file type listed here.
Might be an overkill.
There was a problem hiding this comment.
I thought about that but not sure how benefit we can acquire by declaring it as string literal types. If the benefit is trivial, we can leave it as string for the simplicity of declarations?
There was a problem hiding this comment.
Indeed, spec change and implementation oddity will be more problematic.
| shouldPreload?: (file: string, type: string) => boolean; | ||
| cache?: RenderCache; | ||
| directives?: { | ||
| [key: string]: Vue.DirectiveOptions | Vue.DirectiveFunction |
There was a problem hiding this comment.
Note server side directive will have different implementation. http://ssr.vuejs.org/en/api.html#directives
Line 308 in d333a49
IMHO direcitives will be
[key: string]: (vnode: VNode, dir: VNodeDirective) => voidThere was a problem hiding this comment.
Oops, I totally didn't know about that, thanks 😅
| }, | ||
|
|
||
| directives: { | ||
| example (vnode, directiveMeta) { |
There was a problem hiding this comment.
vnode is inferred as HTMLElement
|
|
||
| export declare function createBundleRenderer(bundle: any, options?: BundleRendererOptions): BundleRenderer; | ||
|
|
||
| type RenderCallback = (err: Error, html: string) => void; |
There was a problem hiding this comment.
err is nullable, if user turns on strictNullChecks TS will wrongly report if (err === null) {} as unreachable error.
(edit: strangely it does not report in 2.3.3)
vue/src/server/create-renderer.js
Line 74 in d333a49
While html is also nullable, I think it is more desirable to declare it as string since users should use it after checking err. Declaring html as nullable will force users to unnecessarily recheck because TS cannot relate two variables in flow control :(
There was a problem hiding this comment.
While html is also nullable, I think it is more desirable to declare it as string
I agree. html should be string for convenience.
| } | ||
|
|
||
| interface BundleRendererOptions extends RendererOptions { | ||
| clientManifest?: any; |
There was a problem hiding this comment.
I'll update this option to object too.
|
👍 so fast :) |
|
wow very fast fix 10x 👍 |
4 participants
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
The PR fulfills these requirements:
devbranch for v2.x (or to a previous version branch), not themasterbranchfix #xxx[,#xxx], where "xxx" is the issue number)Other information:
Added the type declaration file of vue-server-renderer into its
packagedirectory. Also added a brief test code of the declaration.