Add JSDoc types and definitions

[mkarajohn]

Added types to every function and config object, to the best of my ability.

One thing that surfaced from this process is that in some places functions expect arguments as Element, but the functions themselves may have been called in places where the arguments that are passed are of the broader Node type, not Element.

This could lead to bugs theoretically (big if true!); maybe it should get checked, I have placed some TODOs in there to get the reader's attention.

I also ran gen-modules > dist > uglify but I am not sure if I should have or if they are generated automatically.

[1cg]

Hi there. A couple of requests:

• Can you undo the changes to /dist? I'll generate those when we do the next release
• Do you know how to add a type checking task to the npm?

[mkarajohn]

• Can you undo the changes to /dist? I'll generate those when we do the next release

Done

• Do you know how to add a type checking task to the npm?

We will have to install typescript as a developer dependency. We won't be writing the code in TS nor we'll be shipping compiled code (...unless you want to write the source in TS and ship compiled code?); it's only needed in order to parse our JSDoc definitions and validate the code against them. If you are ok with that, we can add something like this

"typecheck": "tsc --allowJs --checkJs --noEmit --target ES5 src/*.js"

What do you say?

[mkarajohn]

For the record I am currently giving it a 2nd pass to clean up type errors that showed up with TS setup locally. I will be pushing a second commit during the day.

[mkarajohn]

Ok, the prevalent issue is that there are lots of cases such as .nextSibling, which return either Node or null values and these values are then passed to functions that expect Element arguments (e.g. the function isSoftMatch clearly expects Elements (or null) as arguments, since internally it tries to access properties that only exist in Elements and not Nodes, but the function itself is called with the return value of something.nextSibling, for example, which can either be Node or null)

Maybe properties such as .nextElementSibling should be used instead?

[mkarajohn]

Ok, a few comments:

• I went ahead and set up the typescript compiler. It is just a dev dependency, it's irrelevant to the final user.
• I fixed up a few more types based on the type errors that were thrown
• Even though you seem to like to reassign values to function arguments, this creates friction when working with types, because your reassignment may be casting the value to a different type than the one defined for the argument (e.g. a stupid example: function(a: number) { a = 'foo'; } --> a is still considered a number, so the reassignment will throw a type error). You should prefer declaring new variables instead of reassigning arguments
• I tried to only make changes to type definitions, and let you handle the type errors as you see fit. (only small change I made was to the mergeDefaults, createMorphContext and insertSiblings).
• In general, in order to avoid type errors, prefer to use set/getAttribute instead of direct assignment.
• You should decide how you want to handle the Node/Element issue. (possible solutions: allow for both in functions that now only accept one of them and make extra runtime checks to ensure type correctness, or, in places where you expect Element but pass Node use the corresponding attributes for getting an Element value (e.g. .nextElementSibling etc)
• Overall, I did not touch the code itself, I left it up to you to decide how to approach type errors. (You can always choose to not type anything apart from the exported values of Idiomorph; this way end users get the types they need and you don't need to bother with types in the internals of the module :eyes: )
• In general the type errors point out instances where bugs could legit show up.

I am here to help with whatever you decide to do

[mkarajohn]

@1cg

Not trying to be a nuisance, but is this still under consideration or should I stop thinking about it? 😂

It's fine if this won't go any further, I just need to have some kind of closure 😅

[1cg]

Hi dimiris no I am still interested in this. I have been mainly focused on htmx 2 though. Will try to get to this next week.

Sent from Proton Mail for iOS

On Fri, Dec 29, 2023 at 8:59 PM, Dimitris Karagiannis @.***(mailto:On Fri, Dec 29, 2023 at 8:59 PM, Dimitris Karagiannis < wrote:

@.***(https://github.com/1cg)

Not trying to be a nuisance, but is this still under consideration or should I stop thinking about it? 😂

It's fine if this won't go any further, I just need to have some kind of closure 😅

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

[1cg]

Still interested in this, but htmx 2.0 is taking up all my time. I promise I will get to it by the end of the month though.

[mkarajohn]

No worries no worries, since you said you have not abandoned it the first time around that was all I wanted to know; don't feel like you have to explain yourself, I realize HTMX 2 is the bigger thing right now :P

On Mon, Jan 8, 2024, 22:51 1cg @.***> wrote:

Still interested in this, but htmx 2.0 is taking up all my time. I promise I will get to it by the end of the month though.

— Reply to this email directly, view it on GitHub https://github.com/bigskysoftware/idiomorph/pull/25#issuecomment-1881801844, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA45DKQTXUMLWTNYAR7NTRTYNRL63AVCNFSM6AAAAABA5ZYUVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBRHAYDCOBUGQ . You are receiving this because you authored the thread.Message ID: @.***>

[1cg]

going to look at this tonight