-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
fix(puppeteer-core): avoid type instantiation errors #9370
fix(puppeteer-core): avoid type instantiation errors #9370
Conversation
Using the accumulator allows enabling the tail-recursion optimization in the TypeScript compiler. Closes #9369
cc @gomain |
🤖 I have created a release *beep* *boop* --- <details><summary>puppeteer: 19.4.0</summary> ## [19.4.0](puppeteer-v19.3.0...puppeteer-v19.4.0) (2022-12-07) ### Features * **chromium:** roll to Chromium 109.0.5412.0 (r1069273) ([#9364](#9364)) ([1875da6](1875da6)), closes [#9233](#9233) ### Dependencies * The following workspace dependencies were updated * dependencies * puppeteer-core bumped from 19.3.0 to 19.4.0 </details> <details><summary>puppeteer-core: 19.4.0</summary> ## [19.4.0](puppeteer-core-v19.3.0...puppeteer-core-v19.4.0) (2022-12-07) ### Features * ability to send headers via ws connection to browser in node.js environment ([#9314](#9314)) ([937fffa](937fffa)), closes [#7218](#7218) * **chromium:** roll to Chromium 109.0.5412.0 (r1069273) ([#9364](#9364)) ([1875da6](1875da6)), closes [#9233](#9233) * **puppeteer-core:** keydown supports commands ([#9357](#9357)) ([b7ebc5d](b7ebc5d)) ### Bug Fixes * **puppeteer-core:** avoid type instantiation errors ([#9370](#9370)) ([17f31a9](17f31a9)), closes [#9369](#9369) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit late to the party, but here goes...
Spotted a minor imperfection, where a couple evaluation branches should be marked as never
.
The Acc
trick does not allow tail recursion optimization (I believe there is no such optimization). It does allow intermediate thunks to be evaluated before the recursive call. This hugely reduces the (virtual) stack size. There still exists a limit on how many tokens the input string can contain before instantiation depth exceeds the limit. From trial and error investigation it seems the recursion depth limit is about 1000 (typescript v4.5.4).
Practically, a selector string with multiple compound selectors with explicit combinators (i.e. not space), such as "div > div > div > ..."
would be limited to ~500 compound selectors. We could probably live with that.
: Acc | ||
: Acc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two branches are never
taken.
Using the accumulator allows enabling the tail-recursion optimization in the TypeScript compiler.
Closes #9369