feat(developer,common): use unified xml parser

Subsystems changed: - ldml keyboard reader (main and test) - kpj - kvks - kmp compiler test: made the test-xml-utils less verbose about the pathnames Fixes: #12208
keymanapp · Sep 27, 2024 · 3832ca1 · 3832ca1
1 parent 7da7375
commit 3832ca1
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 92 deletions.
diff --git a/developer/src/common/web/utils/src/types/kpj/kpj-file-reader.ts b/developer/src/common/web/utils/src/types/kpj/kpj-file-reader.ts
@@ -1,4 +1,4 @@
-import { xml2js } from '../../index.js';
+import { KeymanXMLReader } from '../../index.js';
 import { KPJFile, KPJFileProject } from './kpj-file.js';
 import { util } from '@keymanapp/common-types';
 import { KeymanDeveloperProject, KeymanDeveloperProjectFile10, KeymanDeveloperProjectType } from './keyman-developer-project.js';
@@ -13,20 +13,9 @@ export class KPJFileReader {
   public read(file: Uint8Array): KPJFile {
     let data: KPJFile;
 
-    const parser = new xml2js.Parser({
-      explicitArray: false,
-      mergeAttrs: false,
-      includeWhiteChars: false,
-      normalize: false,
-      emptyTag: ''
-    });
+    data = new KeymanXMLReader({ type: 'kpj' })
+      .parse(file.toString());
 
-    parser.parseString(file, (e: unknown, r: unknown) => {
-      if(e) {
-        throw e;
-      }
-      data = r as KPJFile;
-    });
     data = this.boxArrays(data);
     if(data.KeymanDeveloperProject?.Files?.File?.length) {
       for(const file of data.KeymanDeveloperProject?.Files?.File) {
@@ -126,4 +115,4 @@ export class KPJFileReader {
     util.boxXmlArray(source.KeymanDeveloperProject.Files, 'File');
     return source;
   }
-}
+}
diff --git a/developer/src/common/web/utils/src/types/kvks/kvks-file-reader.ts b/developer/src/common/web/utils/src/types/kvks/kvks-file-reader.ts
@@ -1,5 +1,5 @@
 import { SchemaValidators as SV, KvkFile, util, Constants } from '@keymanapp/common-types';
-import { xml2js } from '../../index.js'
+import { KeymanXMLReader } from '../../index.js'
 import KVKSourceFile from './kvks-file.js';
 const SchemaValidators = SV.default;
 import boxXmlArray = util.boxXmlArray;
@@ -20,31 +20,15 @@ export default class KVKSFileReader {
   public read(file: Uint8Array): KVKSourceFile {
     let source: KVKSourceFile;
 
-    const parser = new xml2js.Parser({
-      explicitArray: false,
-      mergeAttrs: false,
-      includeWhiteChars: true,
-      normalize: false,
-      emptyTag: {} as any
-      // Why "as any"? xml2js is broken:
-      // https://github.com/Leonidas-from-XIV/node-xml2js/issues/648 means
-      // that an old version of `emptyTag` is used which doesn't support
-      // functions, but DefinitelyTyped is requiring use of function or a
-      // string. See also notes at
-      // https://github.com/DefinitelyTyped/DefinitelyTyped/pull/59259#issuecomment-1254405470
-      // An alternative fix would be to pull xml2js directly from github
-      // rather than using the version tagged on npmjs.com.
-    });
-
-    parser.parseString(file, (e: unknown, r: unknown) => {
-      if(e) {
-        if(file.byteLength > 4 && file.subarray(0,3).every((v,i) => v == KVK_HEADER_IDENTIFIER_BYTES[i])) {
-          throw new Error('File appears to be a binary .kvk file', {cause: e});
-        }
-        throw e;
-      };
-      source = r as KVKSourceFile;
-    });
+    try {
+      source = new KeymanXMLReader({ type: 'kvks' })
+        .parse(file.toString()) as KVKSourceFile;
+    } catch(e) {
+      if(file.byteLength > 4 && file.subarray(0,3).every((v,i) => v == KVK_HEADER_IDENTIFIER_BYTES[i])) {
+        throw new Error('File appears to be a binary .kvk file', {cause: e});
+      }
+      throw e;
+    }
     if(source) {
       source = this.boxArrays(source);
       this.cleanupFlags(source);
@@ -197,4 +181,4 @@ export default class KVKSFileReader {
     }
     return 0;
   }
-}
+}
diff --git a/developer/src/common/web/utils/src/types/ldml-keyboard/ldml-keyboard-xml-reader.ts b/developer/src/common/web/utils/src/types/ldml-keyboard/ldml-keyboard-xml-reader.ts
@@ -4,13 +4,12 @@
  * Reads a LDML XML keyboard file into JS object tree and resolves imports
  */
 import { SchemaValidators, util } from '@keymanapp/common-types';
-import { xml2js } from '../../index.js';
 import { CommonTypesMessages } from '../../common-messages.js';
 import { CompilerCallbacks } from '../../compiler-interfaces.js';
 import { LDMLKeyboardXMLSourceFile, LKImport, ImportStatus } from './ldml-keyboard-xml.js';
 import { constants } from '@keymanapp/ldml-keyboard-constants';
 import { LDMLKeyboardTestDataXMLSourceFile, LKTTest, LKTTests } from './ldml-keyboard-testdata-xml.js';
-
+import { KeymanXMLReader } from '@keymanapp/developer-utils';
 import boxXmlArray = util.boxXmlArray;
 
 interface NameAndProps  {
@@ -262,26 +261,9 @@ export class LDMLKeyboardXMLSourceFileReader {
   }
 
   loadUnboxed(file: Uint8Array): LDMLKeyboardXMLSourceFile {
-    const source = (() => {
-      let a: LDMLKeyboardXMLSourceFile;
-      const parser = new xml2js.Parser({
-        explicitArray: false,
-        mergeAttrs: true,
-        includeWhiteChars: false,
-        emptyTag: {} as any
-        // Why "as any"? xml2js is broken:
-        // https://github.com/Leonidas-from-XIV/node-xml2js/issues/648 means
-        // that an old version of `emptyTag` is used which doesn't support
-        // functions, but DefinitelyTyped is requiring use of function or a
-        // string. See also notes at
-        // https://github.com/DefinitelyTyped/DefinitelyTyped/pull/59259#issuecomment-1254405470
-        // An alternative fix would be to pull xml2js directly from github
-        // rather than using the version tagged on npmjs.com.
-      });
-      const data = new TextDecoder().decode(file);
-      parser.parseString(data, (e: unknown, r: unknown) => { if(e) throw e; a = r as LDMLKeyboardXMLSourceFile }); // TODO-LDML: isn't 'e' the error?
-      return a;
-    })();
+    const data = new TextDecoder().decode(file);
+    const source = new KeymanXMLReader({ type: 'keyboard3' })
+      .parse(data) as LDMLKeyboardXMLSourceFile;
     return source;
   }
 
@@ -311,27 +293,8 @@ export class LDMLKeyboardXMLSourceFileReader {
   }
 
   loadTestDataUnboxed(file: Uint8Array): any {
-    const source = (() => {
-      let a: any;
-      const parser = new xml2js.Parser({
-        // explicitArray: false,
-        preserveChildrenOrder:true, // needed for test data
-        explicitChildren: true, // needed for test data
-        // mergeAttrs: true,
-        // includeWhiteChars: false,
-        // emptyTag: {} as any
-        // Why "as any"? xml2js is broken:
-        // https://github.com/Leonidas-from-XIV/node-xml2js/issues/648 means
-        // that an old version of `emptyTag` is used which doesn't support
-        // functions, but DefinitelyTyped is requiring use of function or a
-        // string. See also notes at
-        // https://github.com/DefinitelyTyped/DefinitelyTyped/pull/59259#issuecomment-1254405470
-        // An alternative fix would be to pull xml2js directly from github
-        // rather than using the version tagged on npmjs.com.
-      });
-      parser.parseString(file, (e: unknown, r: unknown) => { a = r as any }); // TODO-LDML: isn't 'e' the error?
-      return a; // Why 'any'? Because we need to box up the $'s into proper properties.
-    })();
+    const source = new KeymanXMLReader({ type: 'keyboard3-test' })
+      .parse(file.toString()) as any;
     return source;
   }
 

diff --git a/developer/src/common/web/utils/src/xml-utils.ts b/developer/src/common/web/utils/src/xml-utils.ts
@@ -14,7 +14,7 @@ export class KeymanXMLReader {
     public constructor(public options: KeymanXMLOptions) {
     }
 
-    public parse(data: string): Object {
+    public parse(data: string): any {
         const parser = this.parser();
         let a: any;
         parser.parseString(data, (e: unknown, r: unknown) => { if (e) throw e; a = r; });

diff --git a/developer/src/common/web/utils/test/test-xml-utils.ts b/developer/src/common/web/utils/test/test-xml-utils.ts
@@ -82,7 +82,7 @@ describe(`XML Reader Test ${GEN_XML_FIXTURES && '(update mode!)' || ''}`, () =>
       for (const path of paths) {
         const xmlPath = makePathToFixture('xml', `${path}`);
         const jsonPath = makePathToFixture('xml', `${path}.json`);
-        it(`read: ${xmlPath}`, () => {
+        it(`read: xml/${path}`, () => {
           // get the string data
           const xml = readData(xmlPath);
           assert.ok(xml, `Could not read ${xmlPath}`);

diff --git a/developer/src/kmc-package/src/compiler/kmp-compiler.ts b/developer/src/kmc-package/src/compiler/kmp-compiler.ts
@@ -1,4 +1,4 @@
-import { xml2js } from '@keymanapp/developer-utils';
+import { KeymanXMLReader } from '@keymanapp/developer-utils';
 import JSZip from 'jszip';
 import KEYMAN_VERSION from "@keymanapp/keyman-version";
 
@@ -180,12 +180,10 @@ export class KmpCompiler implements KeymanCompiler {
 
     const kpsPackage = (() => {
         let a: KpsFile.KpsPackage;
-        let parser = new xml2js.Parser({
-          explicitArray: false
-        });
 
         try {
-          parser.parseString(data, (e: unknown, r: unknown) => { if(e) throw e; a = r as KpsFile.KpsPackage });
+          a = new KeymanXMLReader({ type: 'kps' })
+            .parse(data.toString()) as KpsFile.KpsPackage;
         } catch(e) {
           this.callbacks.reportMessage(PackageCompilerMessages.Error_InvalidPackageFile({e}));
         }