feat(vega-typings): add Typescript Types for vega-loader#4000
feat(vega-typings): add Typescript Types for vega-loader#4000hydrosquall merged 2 commits intomainfrom
vega-loader#4000Conversation
|
I've run both
UpdateMy error was that formatting commands can't be run from the repository root. I should instead do cd packages/vega-typings
yarn format |
c26db54 to
fc44911
Compare
Deploying vega with
|
| Latest commit: |
81b38ac
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dae4f8c2.vega-628.pages.dev |
| Branch Preview URL: | https://cameron-yick-vega-loader-typ.vega-628.pages.dev |
| /** | ||
| * Describes context in which the URI will be used. | ||
| */ | ||
| context: 'href' | 'image'; |
There was a problem hiding this comment.
From code search for how context: ' gets called in the monorepo, I found just 3 sites for href and image.
vega/packages/vega-scenegraph/src/Handler.js
Line 138 in 0b652cd
| * context:dataflow is used when loader is used for getting data into vega-dataflow | ||
| * https://vega.github.io/vega/docs/data/ | ||
| */ | ||
| context: 'dataflow'; |
There was a problem hiding this comment.
Context seems to be exclusively added by vega-dataflow:
vega/packages/vega-dataflow/src/dataflow/load.js
Lines 42 to 43 in 0b652cd
| http: (uri: string, options: any) => Promise<string>; | ||
| load: (uri: string, options?: OptionsWithContext) => Promise<string>; | ||
| sanitize: (uri: string, options: OptionsWithContext) => Promise<{ href: string }>; | ||
| http: (uri: string, options: Partial<RequestInit>) => Promise<string>; |
There was a problem hiding this comment.
Setting RequestInit based on the signature of what fetch would accept:
vega/packages/vega-loader/src/loader.js
Lines 173 to 175 in 0b652cd
| /** | ||
| * Informs loader which context the URI will be used in. | ||
| */ | ||
| type OptionsWithContext = |
There was a problem hiding this comment.
Perhaps rename this to LoaderOptionsWithContext ?
| * Informs loader which context the URI will be used in. | ||
| */ | ||
| type OptionsWithContext = | ||
| | (Partial<BaseLoaderOptions> & { |
There was a problem hiding this comment.
Why are the options optional now but context is not?
There was a problem hiding this comment.
As far as I could see, context was provided every time that Vega is internally calling sanitize or load, so it seemed worthwhile to document that this info is available to switch on
External users of vega-loader aren't required to know about this since it context is not mentioned in the previous README except for images. I wouldn't have known about it if I didn't happen to be console what sorts of data was getting passed to sanitize and load when verifying the types.
I suppose we could mark these types as optional since the code seems to run OK if you use vega-loader without also using it inside of vega, but I figured erring on the side of strictness would be good here. Happy to adjust the type if you'd recommend changing it.
There was a problem hiding this comment.
I think it's fine but I wanted us to be aware of this.
| /** Allows caller to explicitly set loading mode (local or network request). File mode only applies to server-side rendering. */ | ||
| mode: 'file' | 'http'; | ||
| /** Default protocol for protocol-relative URIs, defaults to HTTP */ | ||
| defaultProtocol: string; |
There was a problem hiding this comment.
It looks like this can be file, http, a string that starts with //, or a bunch of other things: https://en.wikipedia.org/wiki/List_of_URI_schemes
I'll include HTTP and file for a start, but leave a string at the end for those other cases.
…es for vega-loader
Changes since v5.31.0 **vega-expression** * Add base64 string encoder/decoder to `vega-expression` and `vega-interpreter` (via #4009). (Thanks @hydrosquall!) **vega-typings** * Add Typescript Types for `vega-loader` (via #4000). (Thanks @hydrosquall!) **docs** * Correct data year citation in dorling-cartogram example (via #4006). (Thanks @dsmedia!) * Update typo in vega.timeFloor description (via #4010). (Thanks @hydrosquall!) * Add Security Advisory Policy for Vega (via #4008). (Thanks @hydrosquall!) * Replace redirect url in `expressions.md` (via #3996). (Thanks @dangotbanned!) * correct queries to query in `crossfilter.md` (via #4005). (Thanks @danmarshall!) --------- Signed-off-by: Lukas Hermann <1734032+lsh@users.noreply.github.com>
Motivation
vega-loader, and decided to move the information from the README to the typings file.anytypes fromLoadertoo!Changes
vega-typingsvega-loader's README does not mention thatsanitizetakes incontextandresponseas additional options - this is something I noticed fromconsole.log'ing the output of the options. For now I only document that in the Typescript types.