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

ts api #1005

Closed
wants to merge 10 commits into from
Closed

ts api #1005

wants to merge 10 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Jul 23, 2022

rebooting #1001 with a "typescript as documentation" approach.

Things to watch for:

  • Plot.valueof(undefined, () => {}) crashed
  • Plot.map("foo") crashed
  • Plot.selectMinX potentially inserted undefined in the facet index
  • mid(x1, x2) was wrongly typed (it expected a data argument)
  • the API for marker passed a Context as {document}, now just passes document: see e891948
  • Plot.stackY crashes on some orders: ("z", "appearance", etc) and X or Y are missing
  • The window transform used the percentile function with the wrong types

? map(data, value, arrayType)
: type === "number" || value instanceof Date || type === "boolean"
: typeof value === "number" || value instanceof Date || typeof value === "boolean"

Choose a reason for hiding this comment

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

I'm seeing warnings where the union of the type is under specified.

Warning:(41, 7) Invalid 'typeof' check: 'value' cannot have type 'number'
Warning:(41, 36) Invalid 'instanceof' check: 'value' has type that is not related to 'Date'
Warning:(41, 61) Invalid 'typeof' check: 'value' cannot have type 'boolean'

A naive fix.. just append said types to the args list. e.g. value: ... | number | Date | boolean,.

Or maybe instead they should be added to the already defined types, e.g. ChannelOption, if not too broad 🤷 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging! I don't get these warnings myself, not in VSCode nor in yarn test:typecheck. How come?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you using the --watch option?

@Fil
Copy link
Contributor Author

Fil commented Jul 26, 2022

This PR would be monstruous to review. I'm leaving it as a draft to show the context, but I'll submit it separately in digestible pieces. The first piece will be data.ts + options.ts

This was referenced Jul 26, 2022
context.ts

src/options

api.ts and options.ts

fix type formatAuto

scales/index

scales/schemes

scales/schemes.ts

src/stats

stats.ts

remove common.ts for now

symbols

symbols.ts

marks/marker

marker.ts; changing the API for marker

the second argument for markers has not been documented yet; changing it from *context* = {document} to *document*

style

style.ts

src/transforms/basic

transform/basic.ts

transform/identity

transforms/identity.ts

transforms/group

transforms/groups.ts

transforms/inset

transforms/inset.ts

transforms/map

transforms/map.ts

transforms/select

transforms/select.ts

transforms/stack

transforms/stack.ts

transforms/interval

transforms/interval.ts

transforms/normalize

transforms/normalize.ts

transforms/window

transforms/window.ts

transforms/bin

transforms/bin.ts

clean-up valueof, map

renames & clean-up on ValueAccessor

valueof cleaner

cleaner defs for data and columns

Add stack error messages when X or Z are missing and the specified order requires them

move data types to src/data.ts

too much typing

README documentation and links for Plot.column, Plot.valueof and Plot.transform (aka basic)

options.ts copied from 6b6aa86a

keep in sync with #1008

follow #1008 on pXX

follow #1008

Datum can also be an array of values, with typical accessors being "length", "0", "1"…

more typings

basic.ts is working

transforms: identity, inset, interval, map

window, and a bit of progress on stack

normalize

select

stack

bin

group

markers and symbols

arrayify comes back

difficulty typing Trasnforms vs Initializer, because Plot.sort applies to either
@Fil Fil mentioned this pull request Aug 8, 2022
@mbostock
Copy link
Member

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

@Fil Fil closed this Mar 14, 2023
@Fil Fil deleted the fil/ts-api branch June 7, 2023 09:11
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