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

TypeScript Declarations #3

Merged
merged 16 commits into from Oct 19, 2022
Merged

Conversation

knexator
Copy link
Contributor

Hello again!
To use Shaku in TypeScript projects, I've found convenient to generate declaration files.
Generating them should be an automatic process by the typescript compiler, but it's not that smooth: some things don't get picked by it (such as the CSS color getters of Color), and other times there are bugs in the compiler (adding the _value property confuses it?? idk). I've made a python script to fix these things.

I've also fixed the typings for enum types: there used to be functions that expected as a param a "MouseButtons", for example, when "MouseButtons" is the enum itself instead of its values. I've fixed this, but note that for consistency I renamed "TextAlignment" to "TextAlignments", which breaks existing code (i've patched the repo examples)

There are still some weird issues (declarations seem to be duplicated or even triplicated; the declaration for createRenderTarget can't pick up that "name" is "String | null" instead of "String"; my fiddling in the python script seems to break the source maps) but overall it's pretty usable.

@knexator
Copy link
Contributor Author

Huh, it seems that typescript projects don't work: either I do import Vector2 from "shaku/lib/utils/vector2";, in which case the type declarations don't work, or I do import Vector2 from "shaku/types/utils/vector2";, in which case it doesn't compile :(

I've read the relevant documentation from package.json, declaration publishing, searched for similar problems (https://stackoverflow.com/questions/72052916/how-to-import-js-module-when-typescript-declaration-file-is-located-in-a-separat). The only solution i've found is to place the .d.ts files all over /lib, but that's muddy and very unclear. Any solutions/suggestions welcome!

@knexator
Copy link
Contributor Author

I've found a workaround: in typescript projects using Shaku, add
"baseUrl": ".", "paths": { "shaku/lib/*": ["./node_modules/shaku/types/*"] }
to tsconfig.json, and import using import Shaku from "shaku/lib/shaku"; import Vector2 from "shaku/lib/utils/vector2"

@RonenNess
Copy link
Owner

Hi @knexator thank you for the additional contribution!
I never used TypeScript so I'll have to trust your judgement on the proposed solution.

I'll check out the changes hopefully this weekend to see if they are acceptable and don't break too much. Will keep you updated.

Thanks :D

@RonenNess
Copy link
Owner

Can you explain more about why the enum def is without the S but the object containing the values is with S and not both the same? For example here:


/** @typedef {String} BlendMode */

/**
 * Blend modes we can draw with, determine how we blend new draws with existing buffer.
 * @readonly
 * @enum {BlendMode}
 */
const BlendModes = {
    AlphaBlend: "alpha",
    Opaque: "opaque",
    Additive: "additive",
    Multiply: "multiply",
    Subtract: "subtract",
    Screen: "screen",
    Overlay: "overlay",
    Invert: "invert",
    Darken: "darken",
    DestIn: "dest-in",
    DestOut: "dest-out"
};

Why did you make the comment / type called BlendMode and not BlendModes like the object itself?

Thanks

@knexator
Copy link
Contributor Author

According to https://jsdoc.app/tags-enum.html, the Type in @enum {Type} is the type of the enum's values, not the type of the enum itself. So the type BlendModes is a constant object, whose values are of type BlendMode (which is actually just a string)

@RonenNess
Copy link
Owner

Ahh I understand now, first you defined a type, BlendMode, which is string. Then you made the enum type be value of BlendMode, aka string but in docs it will have a clearer name.

Sounds legit. I'll merge the pr branch soon, meanwhile feel free to go over my added commits if you got anything to add.

Thanks!

@RonenNess RonenNess merged commit 610db13 into RonenNess:main Oct 19, 2022
@RonenNess
Copy link
Owner

Merged, thanks @knexator! :)

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.

None yet

2 participants