Fix import of modules named with `-`

[LukasHeimann]

In my project, I import a dependency with a - in its name. I don't do this directly, as it is an external non-ui5 library, so I have set up a ui5 shim consisting of a js file and a dts file for the types (eventually moving to the modules tooling is the plan, but here we are):

// unleash.js
sap.ui.loader.config({
  shim: {
    "myapp/lib/unleash-proxy-client/main.min": {
      amd: true,
      exports: "unleash"
    }
  }
});
sap.ui.define(["myapp/lib/unleash-proxy-client/main.min"], unleash => unleash);

// unleash.d.ts
export * from "unleash-proxy-client";

This hasn't caused problems so far, as the dts file is ignored in the babelrc, as per the sample application. Now, with the new karma-ui5-transpile plugin (which works really nice despite this), the following error in the coverage preprocessor of karma pops up:

webapp\lib\unleash.d.ts: Unexpected token, expected "," (3:59)

  1 | "use strict";
  2 |
> 3 | sap.ui.define(["unleash-proxy-client"], function (__unleash-proxy-client) {
    |                                                            ^       
  4 |   var __exports = {
  5 |     __esModule: true
  6 |   };

This means that the babel-plugin-transform-modules-ui5 generates an invalid identifier __unleash-proxy-client for the dependency. I think the issue might be fixed easily by just replacing - with _ in https://github.com/ui5-community/babel-plugin-transform-modules-ui5/blob/main/packages/plugin/src/modules/visitor.js#L7. There is already cleanImportSource which does something like this, but I don't know why this isn't called.

Perhaps this isn't a problem of the babel plugin altogether, but a problem of the karma plugin (which shouldn't try to transpile the dts file), in which case I can also open an issue over at the other repository as well -- please just let me know!

Thank you for the great work with the UI5 typescript support!

[petermuessig]

Hi @LukasHeimann ,

I'll try to fix that the next days. Thanks for reporting and sorry for the late catch-up...

BR, Peter

[LukasHeimann]

No problem, and it isn't hitting us anymore at the moment, as we were able to improve the karma preprocessor glob to "webapp/{!(test)/**/,}!(*.d).ts": ["ui5-transpile"], which excludes the dts file, that isn't relevant for coverage anyways.

I guess, however, the general case still might be an issue, e.g. if we tried to import the dependency directly.

[petermuessig]

Hi Lukas,

just for my information, this was a d.ts file which was transpiled? This shouldn't be the case at all. Did you test it with the latest version of the ui5-tooling-transpile? .d.ts files are excluded in general for the transpilation step: https://github.com/ui5-community/ui5-ecosystem-showcase/blob/main/packages/ui5-tooling-transpile/lib/task.js#L59

Could it be that you also upgraded to the latest versions of ui5-tooling-transpile when starting to use the karma-ui5-transpile?

If this is the case, then this may be the reason for that. In the Babel plugin, I verified today, the dashes are converted properly. There are also some exclusive tests for this.

I think we can close this issue for now.

Cheers, Peter