Add improved TypeScript support. #355
Conversation
|
@domoritz It would be great if you can take a look and chime in on whether this is on the right track and if you'd suggest any changes in approach. |
* chore(deps): bump actions/configure-pages from 4 to 5 (#352) * docs: add types for setPlot and plot attributes --------- Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
@domoritz FYI it looks like your type updates now cause type check failures, probably due to interactions with my changes? |
|
I fixed the issues in my changes so they pass now. I also just enabled type checks for the specs in js. Feel free disable that again but it looks like we have some discrepancies that this finds. E.g. |
Yes, thank you! I've been focusing first on types for the declarative spec itself, and then will look more at the AST node classes. My ideal would likely be to use jsdoc for the classes and |
|
Disabled for now. Might actually be worth switching to Typescript at this point so we type check the implementation against the spec as well. But that can happen later. |
There was a problem hiding this comment.
having both .d.ts and JSDocs with types is confusing and seems to break VSCode at least (and tsk if we type check the js files). The problem is that it doesn't know whether to go to the js file or the d.ts file to get the types for a method etc. It would maybe be easier to either go all in on ts or to move all types into the d.ts files.
Having said that, this is still a good step forwards so I am supportive of merging it.
|
What's your main concern with going all ts? Is it that it would mean that we have to explicitly opt out of typing rather than opt in for all code or that we would need to compile any dependency before we can use it in a dependent package? |
|
I'm not dead set against going all TS at some point. It's just that it's a big porting lift and not a top priority at the moment. |
This PR starts to provide better TypeScript support, but stops well short of complete typings. Sufficient changes were made to prevent the widget test from failing on the latest TS version. Future todos include more coverage of jsdoc-based types in the core Mosaic packages.
Changelog:
tscon them as part of@uwdata/mosaic-spectests.handleParammark utility for better clarity and type-friendliness.