Skip to content

Conversation

@schlawg
Copy link
Contributor

@schlawg schlawg commented Nov 11, 2024

speak SAN followed by "confirm?" when both are enabled

import { throttle } from 'common/timing';
import { defined, charToRole } from 'common';
import { defined } from 'common';
import { speakable } from 'chess/sanWriter';
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesn't that make all pages depend on chessops?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it shouldn't, because it's not grabbing from the 'chess' barrel like it was before. sanWriter only imports types from chessops.

but it's always possible esbuild has decided to bundle them together (unlikely). i will check

Copy link
Contributor Author

@schlawg schlawg Nov 11, 2024

Choose a reason for hiding this comment

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

ok, chessops is not loaded on site (sanWriter is clean).

i did discover it is still loaded by round/ctrl courtesy of keyboardMove dependency - that could be fixed by separating keyboardMove's load stub from initModule. having them in the same bundle defeats the purpose.

on the other hand - meh, it's 37k and with cache disabled, we are only sending around 230 kb (compressed) over the wire total with rounds - and that includes i18n, fonts, and piece css,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was only one function bringing it into chess barrel. removed that and it's no longer fetched in rounds.

now it's only loaded by analysis, puzzles, insights, learn, nvui, storm, racer, keyboardMove, voice, editor, chart, and dgt.

@ornicar ornicar merged commit fe56549 into lichess-org:master Nov 12, 2024
5 checks passed
@schlawg schlawg deleted the ui-speech-confirm branch November 13, 2024 22:11
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.

3 participants