change(web): remove support for es5 🏗️

[ermshiperete]

Fixes: #11878

@keymanapp-test-bot skip

[keymanapp-test-bot]

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

• Android
  • Keyman for Android apk
  • FirstVoices for Android 16.0 apk
  • FirstVoices Keyboards for Android apk
  • KeyboardHarness apk
  • Keyman for Android 16.0 apk
  • KMSample1 apk
  • KMSample2 apk
• Developer
  • Keyman Developer
  • Compiler Regression Tests
  • kmcomp.zip
• iOS
  • Keyman for iOS (simulator image)
  • FirstVoices Keyboards for iOS (simulator image)
  • TestFlight internal PR build version - 18.0.80 (0.11881.11569)
• Keyboards
  • Test Keyboards
• Web
  • KeymanWeb Test Home
• Windows
  • Keyman for Windows
  • FirstVoices Keyboards for Windows
  • Text Editor (32 bit)
  • Text Editor (64 bit)

[jahorton]

I would strongly prefer a specific build folder for tests, then a specific folder for each test set.

Something like build/test/dom/**/*.spec.mjs - this pattern is being maintained for /web/src/test/auto/dom/ tests, so why not be consistent with the pattern and reuse it here?

(While this module's headless tests aren't yet in TS, they could then go under build/test/headless/**.)

A general pattern is for lib folders to hold bundled / fully-compiled outputs, not testing files... right? Or am I just mistaken about that? Either way, that's a pattern I've held to strongly with the lower-level packages; test file outputs do not belong in that folder, in my opinion. Too messy.

[ermshiperete]

Done.

[jahorton]

Note that this file exists to polyfill for things that would be otherwise missing with ES5, etc. There's a strong chance we should be able to eliminate this script entirely, though it'd take a bit of extra work.

(To be clear, that "extra work" could be done later; just make a tracking issue if so.)

[jahorton]

For other reviewers, just in case:

We use eventemitter3 so that Web can safely compile against common/web/types; events is a Node-specific package that won't exist in the browser. There is one particular point of divergence between the two types:

• In events, .emit('error') will actually throw the error if there are no registered listeners.
• In eventemitter3, this specialized behavior is omitted. It will never throw on .emit('error') regardless of listener count.

This is the one spot in xml2js that uses .emit('error'), so we opted for a precise adjustment to accommodate for the difference between the two implementations that will replicate the behavior of the original. One of the common/web/types unit tests fails otherwise due to reliance on the pattern.

[jahorton]

The notes added here should not be considered "blockers" - but they may prove material worth a look. They're a follow-up regarding the noted filesize-increase concerns.

[jahorton]

Requiring this style of import for the utility methods prevents treeshaking from working as well as it could. Having to import the Uni_IsSurrogate* functions through a higher-level structure (the collective export * as util pattern) results in esbuild importing all functions exported from that structure.

Tracking issue: evanw/esbuild#1420

This can be found within the debug build of web's app/browser when built:

Warning - long code block! (click to expand)

  // ../../../../common/web/types/src/util/util.ts
  var util_exports = {};
  __export(util_exports, {
    BadStringAnalyzer: () => BadStringAnalyzer,
    BadStringType: () => BadStringType,
    CONTAINS_QUAD_ESCAPE: () => CONTAINS_QUAD_ESCAPE,
    MATCH_HEX_ESCAPE: () => MATCH_HEX_ESCAPE,
    MATCH_QUAD_ESCAPE: () => MATCH_QUAD_ESCAPE,
    NFDAnalyzer: () => NFDAnalyzer,
    StringAnalyzer: () => StringAnalyzer,
    UnescapeError: () => UnescapeError,
    Uni_IsSurrogate: () => Uni_IsSurrogate,
    Uni_IsSurrogate1: () => Uni_IsSurrogate1,
    Uni_IsSurrogate2: () => Uni_IsSurrogate2,
    boxXmlArray: () => boxXmlArray,
    describeCodepoint: () => describeCodepoint,
    escapeRegexChar: () => escapeRegexChar,
    escapeStringForRegex: () => escapeStringForRegex,
    hexOcts: () => hexOcts,
    hexQuad: () => hexQuad,
    isOneChar: () => isOneChar,
    isPUA: () => isPUA,
    isValidUnicode: () => isValidUnicode,
    toOneChar: () => toOneChar,
    unescapeOne: () => unescapeOne,
    unescapeOneQuadString: () => unescapeOneQuadString,
    unescapeQuadString: () => unescapeQuadString,
    unescapeString: () => unescapeString,
    unescapeStringToRegex: () => unescapeStringToRegex
  });
  function boxXmlArray(o, x) {
    if (typeof o == "object" && !Array.isArray(o[x])) {
      if (o[x] === null || o[x] === void 0) {
        o[x] = [];
      } else {
        o[x] = [o[x]];
      }
    }
  }
  __name(boxXmlArray, "boxXmlArray");
  var MATCH_HEX_ESCAPE = /\\u{([0-9a-fA-F ]{1,})}/g;
  var CONTAINS_QUAD_ESCAPE = /(?:\\u([0-9a-fA-F]{4})|\\U([0-9a-fA-F]{8}))/;
  var MATCH_QUAD_ESCAPE = new RegExp(CONTAINS_QUAD_ESCAPE, "g");
  var _UnescapeError = class _UnescapeError extends Error {
  };
  __name(_UnescapeError, "UnescapeError");
  var UnescapeError = _UnescapeError;
  function unescapeOne(hex) {
    const codepoint = Number.parseInt(hex, 16);
    return String.fromCodePoint(codepoint);
  }
  __name(unescapeOne, "unescapeOne");
  function unescapeOneQuadString(s) {
    if (!s || !s.match(MATCH_QUAD_ESCAPE)) {
      throw new UnescapeError(`Not a quad escape: ${s}`);
    }
    function processMatch(str, m16, m32) {
      return unescapeOne(m16 || m32);
    }
    __name(processMatch, "processMatch");
    s = s.replace(MATCH_QUAD_ESCAPE, processMatch);
    return s;
  }
  __name(unescapeOneQuadString, "unescapeOneQuadString");
  function unescapeQuadString(s) {
    s = s.replaceAll(MATCH_QUAD_ESCAPE, (quad) => unescapeOneQuadString(quad));
    return s;
  }
  __name(unescapeQuadString, "unescapeQuadString");
  function unescapeString(s) {
    if (!s) {
      return s;
    }
    try {
      let processMatch2 = function(str, matched) {
        const codepoints = matched.split(" ");
        const unescaped = codepoints.map(unescapeOne);
        return unescaped.join("");
      };
      var processMatch = processMatch2;
      __name(processMatch2, "processMatch");
      s = s.replaceAll(MATCH_HEX_ESCAPE, processMatch2);
    } catch (e) {
      if (e instanceof RangeError) {
        throw new UnescapeError(`Out of range while unescaping '${s}': ${e.message}`, { cause: e });
      } else {
        throw e;
      }
    }
    return s;
  }
  __name(unescapeString, "unescapeString");
  function hexQuad(n) {
    if (n < 0 || n > 65535) {
      throw RangeError(`${n} not in [0x0000,0xFFFF]`);
    }
    return n.toString(16).padStart(4, "0");
  }
  __name(hexQuad, "hexQuad");
  function hexOcts(n) {
    if (n < 0 || n > 4294967295) {
      throw RangeError(`${n} not in [0x00000000,0xFFFFFFFF]`);
    }
    return n.toString(16).padStart(8, "0");
  }
  __name(hexOcts, "hexOcts");
  function escapeRegexChar(ch) {
    const code = ch.codePointAt(0);
    if (code <= 65535) {
      return "\\u" + hexQuad(code);
    } else {
      return "\\U" + hexOcts(code);
    }
  }
  __name(escapeRegexChar, "escapeRegexChar");
  var REGEX_SYNTAX_CHAR = /^[\u0000-\u001F\u007F-\u009F{}\[\]\\?|.^$*()/+-]$/;
  function escapeRegexCharIfSyntax(ch) {
    if (REGEX_SYNTAX_CHAR.test(ch) || !isValidUnicode(ch.codePointAt(0))) {
      return escapeRegexChar(ch);
    } else {
      return ch;
    }
  }
  __name(escapeRegexCharIfSyntax, "escapeRegexCharIfSyntax");
  function regexOne(hex) {
    const unescaped = unescapeOne(hex);
    return Array.from(unescaped).map((ch) => escapeRegexCharIfSyntax(ch)).join("");
  }
  __name(regexOne, "regexOne");
  function escapeStringForRegex(s) {
    return s.split("").map((ch) => escapeRegexCharIfSyntax(ch)).join("");
  }
  __name(escapeStringForRegex, "escapeStringForRegex");
  function unescapeStringToRegex(s) {
    if (!s) {
      return s;
    }
    try {
      let processMatch2 = function(str, matched) {
        const codepoints = matched.split(" ");
        const unescaped = codepoints.map(regexOne);
        return unescaped.join("");
      };
      var processMatch = processMatch2;
      __name(processMatch2, "processMatch");
      s = s.replaceAll(MATCH_HEX_ESCAPE, processMatch2);
    } catch (e) {
      if (e instanceof RangeError) {
        throw new UnescapeError(`Out of range while unescaping '${s}': ${e.message}`, { cause: e });
      } else {
        throw e;
      }
    }
    return s;
  }
  __name(unescapeStringToRegex, "unescapeStringToRegex");
  function isOneChar(value) {
    return [...value].length === 1;
  }
  __name(isOneChar, "isOneChar");
  function toOneChar(value) {
    if (!isOneChar(value)) {
      throw Error(`Not a single char: ${value}`);
    }
    return value.codePointAt(0);
  }
  __name(toOneChar, "toOneChar");
  function describeCodepoint(ch) {
    let s;
    const p = BadStringAnalyzer.getProblem(ch);
    if (p != null) {
      s = p;
    } else {
      s = `"${String.fromCodePoint(ch)}"`;
    }
    return `${s} (U+${Number(ch).toString(16).toUpperCase()})`;
  }
  __name(describeCodepoint, "describeCodepoint");
  var BadStringType = /* @__PURE__ */ ((BadStringType2) => {
    BadStringType2["pua"] = "PUA";
    BadStringType2["unassigned"] = "Unassigned";
    BadStringType2["illegal"] = "Illegal";
    BadStringType2["denormalized"] = "Denormalized";
    return BadStringType2;
  })(BadStringType || {});
  var Uni_LEAD_SURROGATE_START = 55296;
  var Uni_LEAD_SURROGATE_END = 56319;
  var Uni_TRAIL_SURROGATE_START = 56320;
  var Uni_TRAIL_SURROGATE_END = 57343;
  var Uni_SURROGATE_START = Uni_LEAD_SURROGATE_START;
  var Uni_SURROGATE_END = Uni_TRAIL_SURROGATE_END;
  var Uni_FD_NONCHARACTER_START = 64976;
  var Uni_FD_NONCHARACTER_END = 65007;
  var Uni_FFFE_NONCHARACTER = 65534;
  var Uni_PLANE_MASK = 2031616;
  var Uni_MAX_CODEPOINT = 1114111;
  var Uni_PUA_00_START = 57344;
  var Uni_PUA_00_END = 63743;
  var Uni_PUA_15_START = 983040;
  var Uni_PUA_15_END = 1048573;
  var Uni_PUA_16_START = 1048576;
  var Uni_PUA_16_END = 1114109;
  function Uni_IsSurrogate1(ch) {
    return ch >= Uni_LEAD_SURROGATE_START && ch <= Uni_LEAD_SURROGATE_END;
  }
  __name(Uni_IsSurrogate1, "Uni_IsSurrogate1");
  function Uni_IsSurrogate2(ch) {
    return ch >= Uni_TRAIL_SURROGATE_START && ch <= Uni_TRAIL_SURROGATE_END;
  }
  __name(Uni_IsSurrogate2, "Uni_IsSurrogate2");
  function Uni_IsSurrogate(ch) {
    return Uni_IsSurrogate1(ch) || Uni_IsSurrogate2(ch);
  }
  __name(Uni_IsSurrogate, "Uni_IsSurrogate");
  function Uni_IsEndOfPlaneNonCharacter(ch) {
    return (ch & Uni_FFFE_NONCHARACTER) == Uni_FFFE_NONCHARACTER;
  }
  __name(Uni_IsEndOfPlaneNonCharacter, "Uni_IsEndOfPlaneNonCharacter");
  function Uni_IsNoncharacter(ch) {
    return ch >= Uni_FD_NONCHARACTER_START && ch <= Uni_FD_NONCHARACTER_END || Uni_IsEndOfPlaneNonCharacter(ch);
  }
  __name(Uni_IsNoncharacter, "Uni_IsNoncharacter");
  function Uni_InCodespace(ch) {
    return ch >= 0 && ch <= Uni_MAX_CODEPOINT;
  }
  __name(Uni_InCodespace, "Uni_InCodespace");
  function Uni_IsValid1(ch) {
    return Uni_InCodespace(ch) && !Uni_IsSurrogate(ch) && !Uni_IsNoncharacter(ch);
  }
  __name(Uni_IsValid1, "Uni_IsValid1");
  function isValidUnicode(start, end) {
    if (!end) {
      return Uni_IsValid1(start);
    } else if (!Uni_IsValid1(end) || !Uni_IsValid1(start) || end < start) {
      return false;
    } else if (start <= Uni_SURROGATE_END && end >= Uni_SURROGATE_START) {
      return false;
    } else if (start <= Uni_FD_NONCHARACTER_END && end >= Uni_FD_NONCHARACTER_START) {
      return false;
    } else if ((start & Uni_PLANE_MASK) != (end & Uni_PLANE_MASK)) {
      return false;
    } else {
      return true;
    }
  }
  __name(isValidUnicode, "isValidUnicode");
  function isPUA(ch) {
    return ch >= Uni_PUA_00_START && ch <= Uni_PUA_00_END || ch >= Uni_PUA_15_START && ch <= Uni_PUA_15_END || ch >= Uni_PUA_16_START && ch <= Uni_PUA_16_END;
  }
  __name(isPUA, "isPUA");
  var _BadStringMap = class _BadStringMap extends Map {
    toString() {
      if (!this.size) {
        return "{}";
      }
      return Array.from(this.entries()).map(([t, s]) => `${t}: ${Array.from(s.values()).map(describeCodepoint).join(" ")}`).join(", ");
    }
  };
  __name(_BadStringMap, "BadStringMap");
  var BadStringMap = _BadStringMap;
  var _StringAnalyzer = class _StringAnalyzer {
    constructor() {
      /** internal map */
      __publicField(this, "m", new BadStringMap());
    }
    /** add a string for analysis */
    add(s) {
      for (const c of [...s]) {
        const ch = c.codePointAt(0);
        const problem = this.analyzeCodePoint(c, ch);
        if (problem) {
          this.addProblem(ch, problem);
        }
      }
    }
    /** internal interface for the result of an analysis */
    addProblem(ch, type) {
      if (!this.m.has(type)) {
        this.m.set(type, /* @__PURE__ */ new Set());
      }
      this.m.get(type).add(ch);
    }
    /** get the results of the analysis */
    analyze() {
      if (this.m.size == 0) {
        return null;
      } else {
        return this.m;
      }
    }
  };
  __name(_StringAnalyzer, "StringAnalyzer");
  var StringAnalyzer = _StringAnalyzer;
  var _BadStringAnalyzer = class _BadStringAnalyzer extends StringAnalyzer {
    /** analyze one codepoint */
    analyzeCodePoint(c, ch) {
      return _BadStringAnalyzer.getProblem(ch);
    }
    /** export analyzer function  */
    static getProblem(ch) {
      if (!isValidUnicode(ch)) {
        return "Illegal" /* illegal */;
      } else if (isPUA(ch)) {
        return "PUA" /* pua */;
      } else {
        return null;
      }
    }
  };
  __name(_BadStringAnalyzer, "BadStringAnalyzer");
  var BadStringAnalyzer = _BadStringAnalyzer;
  var _NFDAnalyzer = class _NFDAnalyzer extends StringAnalyzer {
    analyzeCodePoint(c, ch) {
      const nfd = c.normalize("NFD");
      if (c !== nfd) {
        return "Denormalized" /* denormalized */;
      } else {
        return null;
      }
    }
  };
  __name(_NFDAnalyzer, "NFDAnalyzer");
  var NFDAnalyzer = _NFDAnalyzer;

[jahorton]

If I instead import those functions directly from the original, defining source file...

  // ../../../../common/web/types/src/util/util.ts
  var CONTAINS_QUAD_ESCAPE = /(?:\\u([0-9a-fA-F]{4})|\\U([0-9a-fA-F]{8}))/;
  var MATCH_QUAD_ESCAPE = new RegExp(CONTAINS_QUAD_ESCAPE, "g");
  var Uni_LEAD_SURROGATE_START = 55296;
  var Uni_LEAD_SURROGATE_END = 56319;
  var Uni_TRAIL_SURROGATE_START = 56320;
  var Uni_TRAIL_SURROGATE_END = 57343;
  function Uni_IsSurrogate1(ch) {
    return ch >= Uni_LEAD_SURROGATE_START && ch <= Uni_LEAD_SURROGATE_END;
  }
  __name(Uni_IsSurrogate1, "Uni_IsSurrogate1");
  function Uni_IsSurrogate2(ch) {
    return ch >= Uni_TRAIL_SURROGATE_START && ch <= Uni_TRAIL_SURROGATE_END;
  }
  __name(Uni_IsSurrogate2, "Uni_IsSurrogate2");

Way less imported code this way - easily enough to account for a few KB in file-size difference even when minified.

Alternatively, exporting the two explicitly from common/web/types/src/main.ts outside of a "namespace" export would also do the job. Could be "in addition to" to keep the overall change set minimal, rather than removing it from the util namespace currently used by other consumers.

[jahorton]

Alternatively... we could just leave the versions defined within common/web/utils in place, as they're not subject to the import pattern causing the issue.

Alternatively-alternatively, we could expose the util.js file as its own... "sub-export"(?) from common/web/types. It's a pattern I'm comfortable implementing if we want to go that route. We'd then be able to import { whatever } from '@keymanapp/common-types/utils' or similar. (We can choose what we name that final path component.)

[ermshiperete]

Implemented the alternative alternative. With that the size stays the same.

[jahorton]

LGTM, though we should probably get @mcdurdin's thoughts on the parts of the changes that affect common/web/types + developer.

[mcdurdin]

A couple of things I am not comfortable with here. The sharing of code common/web/types between Developer and Web seems still to be a little difficult. Perhaps we should have a discussion about that?

[mcdurdin]

We shouldn't be using deep imports between the kmc modules -- we are working towards well-defined API surfaces for each module, and this is a definite step backwards. Can we please only use top-level imports.

(Doesn't look like eslint has a good rule for this at present?)

[jahorton]

Context: #11881 (comment)

Without this pattern, KMW will grow approximately 4 KB in filesize. That said, yeah, this file isn't part of KMW, so there's no reason to do the deep import here.

[mcdurdin]

I saw the context. I am asking that we don't fix the growth issue by introducing a new problem.

We do not need to use this pattern. We can just export the two functions that KMW needs from common-types/main.ts.

[jahorton]

We do not need to use this pattern. We can just export the two functions that KMW needs from common-types/main.ts.

I do agree with that; it does feel like the better approach.

[ermshiperete]

Done

[jahorton]

It's fine if the constants are the only "extras" along for the ride; their file-size contribution is minimal. This is what I see on my local build - the constants are pulled in, but they're just four extra lines, and the longest part of each is a variable name that can be substantially minified.

Here's the minified form of what we currently get:

var dQ=55296,gQ=56319,uQ=56320,IQ=57343;function vs(Q){return Q>=dQ&&Q<=gQ}l(vs,"Uni_IsSurrogate1");function Ws(Q){return Q>=uQ&&Q<=IQ}l(Ws,"Uni_IsSurrogate2")

Starting with function vs(Q), this is content we need to have anyway - that's the Uni_IsSurrogate1 definition. Before that is a total of... 40 characters. It'd be lovely if we didn't have them, but it's not worth hyper-optimizing.

Oh wait. We actually do use them, too; they're just not inlined. (Inlining would be optimal because each is only used once, but oh well.)

var dQ=55296,gQ=56319, // ...

function vs(Q){return Q>=dQ&&Q<=gQ} // ...

We wouldn't even save 40 chars in total!

[jahorton]

So, to give a concise answer, I pick option 3: neither. Leave it as you currently have it.

[jahorton]

To achieve this I moved the constants to util/consts.ts.

Upon re-reading this, I now think your new point was about the regex constants. They're the only content of the file you're referencing in that quote, after all. That said... I don't see the behavior you're indicating.

Use the following two lines at the top of common/web/types/src/util/util.ts:

import { MATCH_HEX_ESCAPE, CONTAINS_QUAD_ESCAPE, MATCH_QUAD_ESCAPE } from './consts.js';
export { MATCH_HEX_ESCAPE, CONTAINS_QUAD_ESCAPE, MATCH_QUAD_ESCAPE };

The regex constants will still be available for general import where needed in the other packages this way, at their original location. Furthermore, running ./web/build.sh build:app/browser shows no sign of those regex entries within the Web build products when I run it locally.

[mcdurdin]

I'm not seeing the size issue at present -- I think @jahorton's final comment above is probably the best answer (and should resolve the current build failures I hope!)

[ermshiperete]

yes, that works, thanks! Strange that it makes a difference whether or not the consts are defined and exported in the same file...

[mcdurdin]

LGTM

[keyman-server]

Changes in this pull request will be available for download in Keyman version 18.0.80-alpha