keymanapp / keymanapp/keyman

refactor(web): centralize common Transform utilities as distinct Web child project

Open
#14,775 0 comments 0 reactions 1 assignee Claimed by @jahorton View on GitHub
refactor web/
Dominant language
Pascal
Stars
534
Forks
143
Avg merge
2d 10h
Merged PRs (30d)
113

Description

Proposed child project name: `engine/transforms`.

Alternatively, we could add it within the `@keymanapp/web-utils` module.

Over time, we've accumulated a fair number of Transform-focused methods for use - some within the main Web engine, and some within the predictive text worker. In some cases, the same method is re-implemented for each.

I believe that it may be wise to create a centralized child project devoted entirely to Transform utility methods and definitions - especially if and when we decide to address #14709. This way, we DRY out the methods, have a clear centralized location for related unit tests, and ensure that the methods can be easily tree-shaken when needed.

_**Proposed inclusions**_
- `const EMPTY_TRANSFORM = { insert: '', deleteLeft: 0 };`
- This isn't actually a defined constant yet, but is repeated considerably even within the main codebase.
- It's also used quite frequently in many unit test definitions.
- Worker-space:
- web/src/engine/predictive-text/templates/src/common.ts
- `buildMergedTransform`
- `applyTransform`
- web/src/engine/predictive-text/worker-thread/src/main/transformUtils.ts
- `isWhitespace`
- `isBackspace`
- `isEmpty`
- web/src/engine/predictive-text/worker-thread/src/main/transform-subsets.ts
- All types & methods.
- See #14709 - these methods would likely be quite useful within the main engine when addressing the issue
- Main engine:
- web/src/engine/js-processor/src/outputTarget.ts
- `isEmptyTransform` (matches `isEmpty` above)
- `buildTransformFrom` kind of inverts `buildMergedTransform`, computing the Transform between two Contexts. The core of the method can be generalized to work with `Context`s, with `buildTransformFrom` then mapping the Web context types to Context for the operation.

_**Highly-related**_

- Worker-space:
- does not actually have a `buildTransformFrom` analogue.
- Main engine:
- web/src/engine/js-processor/src/outputTarget.ts
- `apply()` is quite analogous to `applyTransform` above, though on the context-wrapping interface instead of on the `Context` type. The interfaces are different enough that it should be kept separately.
- web/src/engine/js-processor/src/mock.ts
- This serves as one of Web's `Context` analogues, though with a significantly different interface and with notable extra functionality (e.g. selected text, deadkeys) attached.
- Usage of a `Mock`, `.apply()` (multiple times), and `.buildTransformFrom` can achieve the effects of `buildMergedTransform` - in which Transform merging happens _on_ the context and is then analyzed later for the total changeset.
- The core could actually be represented according to the standard `Context` interface, with a bit of rework - allowing Mocks to be directly used in methods taking `Context`s.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.