SAP/ui5-tooling

Return explicit ui5 dependencies config

ThePlenkov opened this issue · 10 comments

Hi!

I've noticed that since version 3.0 dependencies now are resolved via package.json.

I just wanted to give you the feedback that this kind of dependency collection will not work in case of monorepos.

If we use monorepo - we maintain all dependencies only on the root level. While in workspaces we use very minimalistic package.json without any dependencies ( they got automatically collected during build )

Moreover in TypeScript based scenario mapping can be resolved via tsconfig.json paths.

That's why the idea to have ui5 middleware dependencies were not bad actually and not that useless. It used probably native require.resolve() and supported all mentioned above techniques.

Additionally - it seems that new way of dependency resolution using ui5 workspaces also doesn't support module paths.

For example:

Error Message:
Failed to resolve workspace dependency resolution path @sap/ux-ui5-tooling to /workspaces/fiori-tools/test/@sap/ux-ui5-tooling: ENOENT: no such file or directory, open '/workspaces/fiori-tools/test/@sap/ux-ui5-tooling/package.json'

I'd suggest that we use relative path only if dot is used. Like ./@sap/ux-ui5-tooling

Hey, thanks for reaching out. I think I understand your use case, but I want to make sure I also understand your setup.

I'm wondering how UI5 Tooling version 2 managed to find all the dependencies in your projects. Did you make use of the "ui5.dependencies" configuration in the package.json files of your workspaces by any chance?

If not, I'm puzzled how this could have worked.

I like the idea of allowing for module names in the workspace configuration. For now, all paths must point to directories though. Have you tried adding a workspace configuration resolving to your root project? Something like this:

specVersion: workspace/1.0
metadata:
    name: default
dependencyManagement:
    resolutions:
        - path: ../

@RandomByte correct. I've been using ui5.dependencies in package.json. I haven't tried yet workspace - because I don't understand what is it for. Is it also for middleware resolution?

So far my workaround is to use * versions in a package.json like here:
https://github.com/theplenkov-npm/fiori-tools/blob/main/test/package.json

while real paths defined here:
https://github.com/theplenkov-npm/fiori-tools/blob/1aa76403b4dd1d7bda89a94e0089505fd8cc6079/tsconfig.base.json#L17

and I run ui5 cli in typescript mode using this trick:

#!/usr/bin/env -S node --loader ts-node/esm
import '@ui5/cli/bin/ui5.cjs';

so it allows me to run node with ts loader and automatically *.js files got resolved to *.ts

But yes - I'd like still prefer to control this manually. Because middleware load may impact some runtime.

It's not convenient to enable/disable extensions by modifying package.json

I haven't tried yet workspace - because I don't understand what is it for. Is it also for middleware resolution?

Indeed, they won't help you in this case as I just remembered myself. Workspaces only allow you to overwrite the resolution of dependencies that are already defined as a project dependency. Currently, project dependencies can only be defined by declaring an npm dependency in the corresponding package.json.

You can learn more about workspaces here: https://sap.github.io/ui5-tooling/stable/pages/Workspace/#dependency-management

So far my workaround is to use * versions in a package.json like here:

This seems to be a good workaround. Did you face any issues with this? Does npm make sure to only install the version maintained in your root package.json or would it install newer versions in the workspaces?


Ultimately, the project needs to tell UI5 Tooling which dependencies it should analyze and ultimately use in a declarative way.

Unfortunately, the "ui5.dependencies" configuration was intended as a workaround in UI5 Tooling version 2, which was not always able to analyze all npm dependencies listed in a package.json (see for example #311). Your use case to analyze dependencies not listed in the package.json was a nice side effect for your use case, but not on our radar. Therefore we removed support for this configuration with UI5 Tooling v3, as it is capable of analyzing all npm dependencies at a good performance.

I would like to refrain from re-adding this configuration option, since it also brings other side-effects and might confuse developers since it's not required anymore for most cases.

The best option I can see to support your scenario would be to make UI5 Tooling aware whenever a project is located inside an npm workspace and therefore should use all dependencies declared in the workspace root as well.

However, we will first have to check whether this is truly npm's assumption too (and Yarn's, pnpm's, etc.). I can imagine that some package managers might make the assumption that all dependencies required for a module, are exclusively defined in the module's package.json.

Npm doesn't install any dependencies from the registry if it's a workspace package. It just creates symlink. Basically it's similar to file: dependency, just no need to care about path.
And what about middleware resolution- it is controlled by typescript of course.

If we check how different frameworks allow extensibility:

almost everywhere plugins are maintained explicitly via config. Having dependency should not mean that this module should be imported automatically.

It's nice also that you have mentioned performance. With a straightforward declaration of middleware list it is not an issue at all, but with automatic scan indeed may appear an issue - especially when we talk about projects with long dependency list

I highly recommend to think of monorepo approach when you will control all the dependencies on a root project level, and package dependencies will be determined during build operation. You will see that in this scenario analyzing dependencies may not work

For UI5 Tooling we decided that the leading source for project dependencies is the package.json file. Since we can identify relevant dependencies easily by checking for the presence of a ui5.yaml file, we wanted to keep it simple for developers and not add another configuration for that.

I can only speculate, but I think that the mentioned frameworks would have a harder time finding out which dependency is a plugin for them, and therefore have to have an explicit configuration.

Our approach requires projects to define all dependencies in the corresponding package.json. Monorepo setups are working well with this, as long as all relevant dependencies are still defined in the workspace project's package.json.

I'm not too familiar with NX, but I would be surprised if there is no mechanism for aligning those dependencies. Certainly a Monorepo can have groups of workspaces that require for example a different version of a dependency than others. How would you solve this?

Have you faced any issues with your workaround of defining the relevant dependencies in package.json using * as the version? Does this behave differently from the old "ui5.dependencies" configuration?

No - it behaves same, but I don't like it anyway. Dependency instruction should not be used as a use instruction to my opinion. We need to enable it somewhere explicitly. Can't we introduce something like .ui5rc file with ability to extend and include plugins?

Similar to what eslintrc is doing or tsconfig

Thank you very much for your feedback. I'm afraid I don't share your opinion on this matter, but I'll bring it up for discussion in our team anyways. This might take a while though due to the current holiday season.

We actually already introduced a .ui5rc file as part of https://github.com/SAP/ui5-tooling/blob/main/rfcs/0013-configuration.md. However, this only contains general (project independent) UI5 Tooling configuration.

I think if we were to add an explicit dependency declaration that overrules the package.json dependency, we should do so in the ui5.yaml.