Handling Circular Dependencies

[texttechne]

Hi @petermuessig,

whenever custom TypeScript sources contain circular dependencies (e.g. A imports B and B imports A), then any UI5 app will throw an exception, saying that one of those dependencies is undefined.

The reason is that all import statements are transformed to sap.ui.define which cannot handle circular dependencies.
Using sap.ui.require would work in those cases.

One might argue that circular dependencies should be prevented in the first place, but I have an example (in combination with odata2ts) for which circular dependencies are a valid use case (actually, OData allows for that by virtue of its navigation model).

Workaround: Dynamic Imports

The workaround that would come to mind is using dynamic imports in the custom TS files which would then be transpiled to sap.ui.require.

However, that would be really cumbersome and intransparent for users... in my case, generating TS artefacts, it's additionally hard to nigh impossible.

Example App

Here's an example app

[petermuessig]

Hi @texttechne ,

I checked the behavior of ES modules and found out that they also run into a similar error, that a module cannot be accessed before it is being initialized:

I created a PR to reproduce this behavior:
texttechne/ui5-circular-deps-example#1

npm run test-esm

As mentioned in the chat, I will discuss this with my colleagues. We had to do similar things for the UI5 Web Components and the solution was to add configuration which defines in which module the static import needs to be converted to a dynamic import. But best would be for sure, if the source modules use a dynamic import which will be then converted properly into a sap.ui.require call.

Hmm, but well - you argue with the valid scenario. Well, yes - but I'm not sure whether this can be solved automatically always... It will definitely produce some ugly code... And it always needs to have the full context. In case of transpiling TS files on-the-fly for the UI5 middleware case, this is not the case and this would mean it needs to read the workspace then and also consider this for dependencies (which could also be transpiled on-the-fly when configured)...

[texttechne]

Hi @petermuessig,
I've updated the example once again (after merging your PR): It works in ESM if it's not a direct top reference, so I've changed the example to use functions which make use of the imported stuff instead of using it in top level variables. As I said, now it works in ESM, but still fails with UI5. Just as a more complete example....

I also discussed this again with my colleagues. I've come to the conclusion, that I should bundle all the generated files (well, the services) into one file. This would solve the problem of circular dependencies... Not nice and a lot of work (a lot of my tests depend on the single file stuff), but I don't see any other way forward....

If you find a magical solution with your colleagues I'm all ears, but per se it sounds like it has to be solved in user land....

[petermuessig]

Hi @texttechne ,

yes the bundling would be the easiest solution. Let me think about this and discuss it also on my side. One option would be that in such cyclic dependency cases, all places referencing the other module from the cyclic dependency needs to remove the other module from the sap.ui.define(["./A"], function(A) { ... }) and use the sap.ui.require("./A") with the string value to resolve an already loaded module from the module loader. But therefore we would need to know the cycle and the middleware needs more context which would come with higher costs. Let me think about this a bit more before changing...

[codeworrior]

@petermuessig removing the dependencies from sap.ui.define IMO would be wrong or counter productive. Only access needs to be rewritten to a probing require, ideally combined with caching to avoid too many require calls.

If the tooling sees all sources that form the cycle in a single run, it should even be possible to detect this automatically. But if gathering such "global" knowledge about all sources is a problem in the bable tool chain, then making this a mandatory config also would be an option (less attractive, however).

[petermuessig]

@codeworrior what I noticed with the example used above is that the UI5 module loader detects the cycle and then fails to load the module and thus the complete app stops working. The ES modules continue to work having the cyclic dependencies most probably because of the rebinding of the exports (or however this is called). To make the example above running in UI5 AMD-like the only option is to remove the cyclic dependency in one of the modules from the sap.ui.define.

Regarding the context - it is not impossible to do so - but so far we only compile/transpile the requested resource without any check of the dependencies used. This would be also some logic on top of the pure transpile run.

[texttechne]

Hi,

in the meantime I've solved the issue for my use case by generating artefacts into the same file.
This solves the issue for me: Special case, but with some effort it's solvable in user land...

@petermuessig do you want to keep the issue?

[petermuessig]

Hi @texttechne , sorry for being late to the party... I close the issue for now - still not sure whether it is user-code-related or we find a better solution - maybe I can do so with the re-architecture of the plugin.

Related #101