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 definition changes and explaining Sinuous internals #114

Closed
wants to merge 9 commits into from

Conversation

nettyso
Copy link
Contributor

@nettyso nettyso commented Jun 8, 2020

I've been trying to understand how Sinuous works a bit better since I'm writing taping into the API calls, mostly api.add, to have tracing of its execution. Running into issues where it's hard to understand how data flows through. Some of the JSDoc types aren't as details as they could be, for instance, I believe api.add(parent, value, endMark) could be given a DocumentFragment or an Array as a value but the JSDoc just says "Node" (I understand that a fragment is an instance of Node but it's not super clear).

Update: I actually finished the project that prompted all these commits/PRs into Sinuous. I was trying to add onAttach/onDetach hooks without using MutationObserver and needed to better understand how api.add/insert/h worked but I figured it out in the end. Added some comments in this PR for anyone who comes after me...

@@ -11,31 +11,34 @@ declare namespace _h {
Record<string, unknown>
| null,
...children: ElementChildren[]
): HTMLElement;
): HTMLElement | SVGElement;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We merged this file earlier in my ts-work branch but really I'm lying to people to say it's only HTMLElement in h/index.d.ts - It's true that later on in src/index.d.ts and jsx/index.d.ts the types are redefined to be more narrow, but until then it's good to define the reviver as generic for both HTML/SVG

property(el: Node, value: unknown, name: string, isAttr?: boolean, isCss?: boolean): void;
add(parent: Node, value: Node | string, endMark?: Node): Node;
add(parent: Node, value: Value | Value[], endMark?: Node): Node | Frag;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So... Frag is mostly an internal concept, but for anyone writing wrappers (aka anyone actually bothering to touch api.add) it's probably good to mention

@codecov
Copy link

codecov bot commented Jun 8, 2020

Codecov Report

❗ No coverage uploaded for pull request base (types-api@d822293). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             types-api     #114   +/-   ##
============================================
  Coverage             ?   97.44%           
============================================
  Files                ?       22           
  Lines                ?      626           
  Branches             ?        0           
============================================
  Hits                 ?      610           
  Misses               ?       16           
  Partials             ?        0           
Impacted Files Coverage Δ
packages/sinuous/h/src/add.js 100.00% <ø> (ø)
packages/sinuous/h/src/api.js 100.00% <ø> (ø)
packages/sinuous/map/src/map.js 100.00% <0.00%> (ø)
packages/sinuous/map/mini/src/mini.js 100.00% <0.00%> (ø)
packages/sinuous/render/src/index.js 100.00% <0.00%> (ø)
packages/sinuous/hydrate/src/hydrate.js 100.00% <0.00%> (ø)
packages/sinuous/data/src/data.js 100.00% <0.00%> (ø)
packages/sinuous/map/src/diff.js 100.00% <0.00%> (ø)
packages/sinuous/memo/src/memo.js 100.00% <0.00%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d822293...be56557. Read the comment docs.

nettyso added 6 commits June 7, 2020 21:50
Else SVG components are said to return DocumentFragment because they
resolve to the last overload of `hs`
- h(Function, ...) and h([...], ...) can return fragments
- sinuous/src's h() is not the same as sinuous/h's api.h()

...Except they are. It depends on api.s. Hard to define
@nettyso
Copy link
Contributor Author

nettyso commented Jun 10, 2020

Updated the description of the PR. I finished the project (added lifecycles! https://nthm.gitlab.io/stayknit/) I was doing that started all these commits, so I won't keep bothering you :)

@luwes luwes closed this Jun 20, 2020
@nettyso nettyso deleted the types-api branch June 25, 2020 02:21
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.

2 participants