Compile everything to ES5

[neilius]

In trying to implement some custom annotations, I came up against the dreaded Class constructor ... cannot be invoked without 'new' error when trying to subclass the BlockAnnotation. After beating my head against the wall trying all kinds of different Babel / webpack configs and a number of discussions with @tim-evans, I think this is the way forward to ensure the widest range of compatibility for the library generally, and it will conveniently fix the issues I'm coming up against when creating custom annotations.

Please discuss and let me know if you think this is a bad idea or if you have concerns with this approach!

[neilius]

This is now hardcoded instead of relying on the superclass

[neilius]

This mentions that it is generated automatically, where / how? Do I need to update anything beyond what I've already done to ensure this won't get wiped out in the future?

[colin-alexa]

it's generated by a script we run manually. I don't anticipate us needing to run it again anytime soon? it just scrapes the html tag spec to generate annotation classes and like...unless they add a bunch of new tags I think we'd probably do any updates by hand

[tim-evans]

These are the lines to change FWIW:

https://github.com/CondeNast/atjson/blob/latest/packages/%40atjson/source-html/scripts/generate-annotations.ts#L216-L218

[neilius]

URLSearchParams needs these to be treated as iterable

[tim-evans]

The HTML annotations are generated by code in the scripts directory

[tim-evans]

Comparing commonmark-spec: current against baseline

There's a 99.99% confidence that the time to execute this benchmark increased by 223.183ms ~ 640.693ms.

name	location	before	after	confidence
createAnnotation	document document.js	21051	10391	-19629, -1692
set	document collection.js	12795	5104	-11957, -3426
splitDelimiterRuns	renderer-commonmark index.js	3339	1079	-3886, -634
without	document collection.js	3227	1381	-3599, -93
getNextChar	renderer-commonmark index.js	1000	0	-1877, -123

---

Comparing commonmark-spec equality: current against baseline

There's a 99.99% confidence that the time to execute this benchmark increased by 76.683ms ~ 163.991ms.

name	location	before	after	confidence
createAnnotation	document document.js	43780	22865	-28398, -13432
set	document collection.js	9046	4665	-8565, -197
Annotation	document annotation.js	15078	10820	-7337, -1177
ParseAnnotation	document annotations/parse.js	6151	2820	-5440, -1222
ConversionDocument	document document.js	4159	1824	-4030, -640
without	document collection.js	2078	833	-2351, -139

---

Comparing degenerate-markdown: current against baseline

There's a 99.999% confidence that the time to execute this benchmark increased by 474.560ms ~ 798.350ms.

name	location	before	after	confidence
createAnnotation	document document.js	71098	43163	-36649, -19223
set	document collection.js	30256	2632	-34747, -20500
Annotation	document annotation.js	16747	11902	-8910, -780
ParseAnnotation	document annotations/parse.js	9337	5160	-7067, -1286
cloneAnnotation	document document.js	7690	4540	-5667, -633

---

Comparing degenerate-markdown equality: current against baseline

We're seeing with ~99.999% confidence that there is an increase in time between 506.480ms ~ 856.267ms, which isn't shown in the table below. We'll try to track down what functions became slower, but this is expected due to changes in the underlying data structures

name	location	before	after	confidence
createAnnotation	document document.js	98758	67654	-43482, -18726
set	document collection.js	34749	2853	-34790, -29003
Annotation	document annotation.js	29371	16586	-17963, -7606
ParseAnnotation	document annotations/parse.js	18012	7829	-13516, -6849
cloneAnnotation	document document.js	13384	5882	-10648, -4354
InlineAnnotation	document annotations/inline.js	5171	3254	-3594, -242

[bachbui]

Note for the above: the ES5 compilation has caused a lot of function names to change from the point of view of the profiler, so we should ignore all of the function stats listed above. The complete baseline comparison (baseline-current-tStats.json) lists numerous functions being dropped and then added under a slightly different name. We can only really go by the cumulative time for the test runs which consistently show a degradation in performance.

[neilius]

Well that's not great 😞

[tim-evans]

@neilius it's to be expected FWIW. We're still working on optimizations, but may need to change what we optimize. I think we can fiddle with some more tsconfig.json options to improve what polyfills they use

[tim-evans]

Ahh, it's probably the lib property that we can futz with to include the absolute minimal amount of polyfills.

[tim-evans]

@neilius and I are going to try to figure out a different way forward for this that doesn't involve taking performance for everyone.

[neilius]

I'm closing this in favor of an alternate approach that we'll handle on internal repos.