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

Basic JSDoc types - Plot.plot and all Marks #1055

Closed
wants to merge 23 commits into from

Conversation

duaneatat
Copy link
Contributor

@duaneatat duaneatat commented Sep 23, 2022

This PR uses a types.ts file for authoring the types (this is just for convenience, since I find it easier to write typescript types than use the JSDoc syntax), and we import those types as jsdoc tags where appropriate.

This PR covers Plot.plot and all of the marks.

Known issues:

  • Some types are too broad, e.g. tickX doesn't accept x1 and y1, but those appear in the completions

TODO:

  • Plot.image : move the description of options from the ### Image section to the Plot.image jsdocs

@duaneatat duaneatat requested review from Fil and mbostock September 23, 2022 13:34
README.md Outdated Show resolved Hide resolved
@duaneatat duaneatat marked this pull request as ready for review September 26, 2022 20:47
@@ -91,17 +101,13 @@ function getJsDocsForFunction(name: string, declaration: FunctionDeclaration, pr
for (const overload of overloads) {
const docs = overload.getJsDocs();
if (!docs.length) continue;
parts.push(docs.map((doc) => makeRelativeUrls(doc.getDescription())).join("\n\n"));
parts.push(transformDocs(docs));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the changes to jsdoc-to-readme are about trimming the JSDoc so extra leading and trailing new lines don't end up in the README file. For some reason, when you have a JSDoc in a file that is just importing types, TypeScript returns empty lines for that JSDoc. This is benign but will result in a slightly cleaner README file.

@Fil
Copy link
Contributor

Fil commented Sep 28, 2022

I think I'd like to have all the types imports at the top of each file, as I was trying to do with the "import types" in #1005.

Maybe for later: I've tried to add a BarYOptions to the barY(data, options) mark, by creating

export interface BarYOptions extends MarkOptions {
  offset: null | "expand" | "center" | "wiggle" | function…;
  order?: "sum" | "value" | …;
}

Of course, it would have to be extended to cover all the stacking options, which means I'd want to write something like BarYOptions = MarkOptions & StackYOptions;. Not sure if this can be done with "type" or "interface", though.

@Fil
Copy link
Contributor

Fil commented Sep 30, 2022

[reminder for next steps:] I've just noticed that Plot.window(X/Y) and Plot.normalize(X/Y) have no jsdocs at all.

@duaneatat duaneatat changed the title Basic JSDoc types Basic JSDoc types - Plot.plot and all Marks Oct 3, 2022
Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

src/marks/area.js Outdated Show resolved Hide resolved
@@ -2346,7 +2346,6 @@ Plot.rect(athletes, Plot.bin({fillOpacity: "count"}, {x: "weight", y: "height"})
Bins on *x* and *y*. Also groups on the first channel of *z*, *fill*, or
*stroke*, if any.


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbostock
Copy link
Member

Probably superseded by #1320 which goes the d.ts route. Close?

@Fil
Copy link
Contributor

Fil commented Mar 13, 2023

I'm good with closing, but let's keep the branch around for a while. There are a few things that I'll want to refer to, for example the to replace the D3 type:

/**
 * A symbol object with a draw function
 * @link https://github.com/d3/d3-shape/blob/main/README.md#custom-symbol-types
 */
type SymbolObject = {draw: (context: CanvasPath, size: number) => void};

@Fil Fil marked this pull request as draft March 13, 2023 20:11
@mbostock
Copy link
Member

mbostock commented Apr 1, 2023

Superseded by #1343.

@mbostock mbostock closed this Apr 1, 2023
@duaneatat duaneatat deleted the duane/basic-jsdoc-types branch August 24, 2023 17:20
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.

3 participants