Slow type checking with snabbdom 3.6+

[jonbgamble]

Recent releases after 3.5.1 seem to really trouble tsc.

We import and use snabbdom from ~ 31 modules of a monorepo.

Our type checking takes 3x as long when those modules import snabbdom 3.6 vs 3.5.1.

48s vs 14s

[paldepind]

Hi @schlawg. Thanks for reporting this. That is quite a noticeable slowdown. Would it be possible for you to git bisect this to figure out what caused the issue?

One thing I can think of regarding typing is that the types for JSX was improved in https://github.com/snabbdom/snabbdom/pull/1092.

[paldepind]

Alternatively, can you provide a way to reproduce this issue? Then I can do the bisect and try to figure out what caused this.

[jonbgamble]

Hello there. Sorry for the delay in responding.

I can't spare any cycles to make an MRE at the moment, but I do have steps.

git clone https://github.com/lichess-org/lila
cd lila && ui/build -c --tsc

Note the time. Now do a global search/replace in ui folder, changing every "snabbdom": "3.5.1" to "snabbdom": "3.6.2" in package.json files.

ui/build -c --tsc again and see it takes roughly 3x as long with 3.6.x. It's probably some bizarre interaction with our codebase (and we're fine on 3.5.1 fyi)

[paldepind]

Thanks a lot @schlawg. That seems like it should be enough to bisect the issue. I'll take a look when I have the time.

[jonbgamble]

Hello @paldepind

Your instincts were correct and it's to do with the VNodeData for jsx. I made an MRE for you here https://github.com/schlawg/snabbdom-tsc-mre

I'm sorry it took me so long. I will try not to take this excellent library for granted in the future!

[paldepind]

Thanks for the reproduction @schlawg. I can see the performance difference and will look into this.

[paldepind]

Please check out this PR: https://github.com/snabbdom/snabbdom/pull/1127.

I'd be very interested in whether this performance improvement is good enough for you (I think the easiest way to test it is to checkout the branch and locally install it with npm i ../path/to/snabbdom/).

If this is still too slow I think we should revert the style changes done in https://github.com/snabbdom/snabbdom/pull/1092.

[jonbgamble]

Thanks Simon! Looks like an improvement, but unfortunately not for our project. The { style: { color: 'red' } } in my MRE demonstrates a similar slowdown to what we're seeing, but it also could be a bit of a red herring (for our specific issue) as we don't use VNodeStyle typings. (sorry).

We have no jsx/tsx sources, but repeated evaluation of computed types in snabbdom/build/jsx.d.ts seem to be causing our slowdowns.

I did a tsc trace of this folder, one of many in our monorepo. With 3.5.x snab, the trace generates a types.json that is under 5MB and looks normal. When I do tsc trace using snabbdom 3.6.2, it balloons to 66MB composed mostly of the block below repeated 20,000+ times:

{"id":65518,"symbolName":"__type","recursionId":107,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":32,"character":32},"end":{"line":35,"character":8}},"flags":["Object"]},
{"id":65519,"symbolName":"Q","recursionId":106,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":34,"character":10},"end":{"line":34,"character":16}},"flags":["TypeParameter","IncludesMissingType"]},
{"id":65520,"symbolName":"__type","recursionId":113,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":35,"character":9},"end":{"line":38,"character":8}},"flags":["Object"]},
{"id":65521,"symbolName":"Q","recursionId":112,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":37,"character":20},"end":{"line":37,"character":26}},"flags":["TypeParameter","IncludesMissingType"]},
{"id":65522,"symbolName":"__type","recursionId":101,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":30,"character":6},"end":{"line":30,"character":34}},"flags":["Object"],"display":"<T>() => T extends { append: (...nodes: (string | Node)[]) => void; } ? 1 : 2"},
{"id":65523,"symbolName":"__type","recursionId":114,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":30,"character":43},"end":{"line":30,"character":72}},"flags":["Object"],"display":"<T>() => T extends { append: (...nodes: (string | Node)[]) => void; } ? 1 : 2"},
{"id":65524,"symbolName":"T","recursionId":100,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":30,"character":45},"end":{"line":30,"character":46}},"flags":["TypeParameter","IncludesMissingType"]},
{"id":65525,"symbolName":"T","recursionId":99,"firstDeclaration":{"path":"/home/gamblej/ws/lichess/snabbdom/build/jsx.d.ts","start":{"line":30,"character":7},"end":{"line":30,"character":8}},"flags":["TypeParameter","IncludesMissingType"]},
{"id":65526,"recursionId":115,"conditionalCheckType":65525,"conditionalExtendsType":65518,"conditionalTrueType":142,"conditionalFalseType":144,"flags":["Conditional","IncludesEmptyObject"]},
{"id":65527,"recursionId":116,"conditionalCheckType":65524,"conditionalExtendsType":65520,"conditionalTrueType":142,"conditionalFalseType":144,"flags":["Conditional","IncludesEmptyObject"]},
{"id":65528,"recursionId":116,"conditionalCheckType":121,"conditionalExtendsType":65520,"conditionalTrueType":142,"conditionalFalseType":144,"flags":["Conditional","IncludesEmptyObject"]},
{"id":65529,"recursionId":115,"conditionalCheckType":121,"conditionalExtendsType":65518,"conditionalTrueType":142,"conditionalFalseType":144,"flags":["Conditional","IncludesEmptyObject"]},

Maybe tsc gets overwhelmed evaluating the IfEquals -> WritableKeys -> ElementProperties -> ElementProperties -> VNodeProps -> HtmlElements -> IntrinsicElements chain recursively for every possible key in HTMLElementTagNameMap over and over. Renaming the jsx namespace & types to foo, bar, etc in jsx.d.ts has no effect, typechecking is still hosed. I'm not sure whether that rules out namespace pollution as a contributing factor.

Perhaps tsc simply parses jsx.d.ts many times when compiling the ~120 sources in our folder, and so the repeated evaluation of the computed types for every html tag just kind of takes over.

[paldepind]

Thanks Simon! Looks like an improvement, but unfortunately not for our project. The { style: { color: 'red' } } in my MRE demonstrates a similar slowdown to what we're seeing, but it also could be a bit of a red herring (for our specific issue) as we don't use VNodeStyle typings. (sorry).

I see. Then it's definitely a red herring. In that example reverting the changes to VNodeStyle in #1092 completely recovered the old speed of type-checking, so the other parts of #1092 does not affect that example at all.

And just to be sure, you don't actually use any of the types that where changed in #1092?

Given that your project is open source, I think it's probably better that I try building that, as a benchmark and as you suggested earlier. I'm getting a bunch of type errors when I run ui/build -c --tsc:

20:05:46 [tsc] - ✘ [ERROR] ts6053 in '@types/lichess/index.d.ts:5:23' - File '.../lila/ui/@types/lichess/i18n.d.ts' not found.

Is that expected? Would it be possible to just run tsc in some directory instead?

[jonbgamble]

Not sure if it's relevant, but you can observe that same 3.5MB -> 70 MB blowup of the generated types.json file using tsc --generateTrace traceDir with snab 3.5.x vs snab 3.6.x on the following:

// example.ts
import { h } from 'snabbdom';

export x = h('div');

[jonbgamble]

here are better instructions for manual tsc inside a lichess monorepo subfolder (such as ui/analyse).

first, update to latest master (for https://github.com/lichess-org/lila/commit/ae25bc23e40cd1bb0c7bacd630bb8ce8ca2a9c8e)

then, before each manual tsc run, do:

../build -c --i18n

we haven't focused on the tsc via cli workflow for some time, so apologies for the hassle.

[kraktus]

bump, any way you'd need help?

[paldepind]

It would be great to have a way to reproduce the performance issue independently of the lila repo. Once we have that, I think we should just revert https://github.com/snabbdom/snabbdom/pull/1092. We could also do the revert without a reproducer, but then we wouldn't have insurance that a similar regression wont happen again.