keyman icon indicating copy to clipboard operation
keyman copied to clipboard

chore(common): ldml-keyboard-xml-reader uses `import.meta.url` but we need to be passing paths from caller

Open mcdurdin opened this issue 1 year ago • 1 comments

The following should not be compiled in:

https://github.com/keymanapp/keyman/blob/5dbda001fcd8973b4922292b5c0731748380a263/common/web/types/src/ldml-keyboard/ldml-keyboard-xml-reader.ts#L24-L26

The imports path cannot have a default because we cannot rely on the files being available from the package anyway, and as we can see, some build environments can't use it anyway.

This will be less critical with the split of common/web/types to move developer source file types into a developer package (#9665), but as a matter of policy and consistency we need to do it anyway.

[web/src/app/browser] ## build starting...
▲ [WARNING] "import.meta" is not available in the configured target environment ("es2015") and will be empty [empty-import-meta]

    ../../../../common/web/types/src/ldml-keyboard/ldml-keyboard-xml-reader.ts:25:26:
      25 │     return ['../import/', import.meta.url];
         ╵                           ~~~~~~~~~~~

▲ [WARNING] "import.meta" is not available in the configured target environment ("es2015") and will be empty [empty-import-meta]

    ../../../../common/web/types/src/ldml-keyboard/ldml-keyboard-xml-reader.ts:25:26:
      25 │     return ['../import/', import.meta.url];
         ╵                           ~~~~~~~~~~~

▲ [WARNING] "import.meta" is not available in the configured target environment ("es2015") and will be empty [empty-import-meta]

    ../../../../common/web/types/src/ldml-keyboard/ldml-keyboard-xml-reader.ts:25:26:
      25 │     return ['../import/', import.meta.url];
         ╵                           ~~~~~~~~~~~

▲ [WARNING] "import.meta" is not available in the configured target environment ("es2015") and will be empty [empty-import-meta]

    ../../../../common/web/types/src/ldml-keyboard/ldml-keyboard-xml-reader.ts:25:26:
      25 │     return ['../import/', import.meta.url];
         ╵                           ~~~~~~~~~~~

mcdurdin avatar Jan 11 '24 02:01 mcdurdin

@mcdurdin so, the callers (two tests, and two sites within kmc) should each have their own definition of this, and remove this function? (maybe combine the two kmc callers into one).

common/web/types/test/helpers/reader-callback-test.ts:
  11  const readerOptions: LDMLKeyboardXMLSourceFileReaderOptions = {
  12:   importsPath: fileURLToPath(new URL(...LDMLKeyboardXMLSourceFileReader.defaultImportsURL))
  13  };

developer/src/kmc/src/commands/buildClasses/BuildLdmlKeyboard.ts:
  13      const ldmlCompilerOptions: kmcLdml.LdmlCompilerOptions = {...options, readerOptions: {
  14:       importsPath: fileURLToPath(new URL(...LDMLKeyboardXMLSourceFileReader.defaultImportsURL))
  15      }};

developer/src/kmc/src/commands/buildTestData/index.ts:
  19      readerOptions: {
  20:       importsPath: fileURLToPath(new URL(...LDMLKeyboardXMLSourceFileReader.defaultImportsURL))
  21      }

developer/src/kmc-ldml/test/helpers/index.ts:
  39    readerOptions: {
  40:     importsPath: fileURLToPath(new URL(...LDMLKeyboardXMLSourceFileReader.defaultImportsURL))
  41    }

srl295 avatar Jun 20 '24 21:06 srl295

so, the callers (two tests, and two sites within kmc) should each have their own definition of this, and remove this function? (maybe combine the two kmc callers into one).

Yep, that sounds about right

mcdurdin avatar Jul 05 '24 07:07 mcdurdin

👉 start this on top of the 🐉 chain @ #12101

srl295 avatar Aug 06 '24 02:08 srl295

Sentry Issue: KEYMAN-DEVELOPER-251

sentry[bot] avatar Aug 06 '24 15:08 sentry[bot]

per https://github.com/keymanapp/keyman/pull/12108#issuecomment-2272549431 closing this

srl295 avatar Aug 07 '24 19:08 srl295