Move away from @ng-dynamic-forms

[ybnd]

Issue / context

During the Angular 15 upgrade, @Atmire-Kristof noticed conflicts between the newer versions of the following libraries:

• ng-bootstrap
  • v14 supports Angular 15 properly
• @ng-dynamic-forms
  • v16+ claims to support Angular 15, but @ng-dynamic-forms/ui-ng-bootstrap still requires ng-bootstrap v11 as a peer dependency, which itself doesn't support Angular 15
  • After "forcing" the upgrade, builds fail because of inconsistencies between the ng-bootstrap versions, so this is not a viable workaround

Rather than do a half-upgrade we opted to leave both of these libraries as-is for now. This leads to peer dependency mismatch warnings, but everything remains functional.

Solution

It looks like development of @ng-dynamic-forms has slowed down as of late, and this version mismatch is a bit of a red flag. Therefore, we should consider moving away from this library and use the default Angular forms instead for more stability.

This would involve a significant refactor of DSpace Angular submission.

References

Add references/links to any related issues or PRs. These may include:

• Related to [Submission Refactor] Refactor Entity Relationships to be modified via WorkspaceItem #858

• Left out of Angular 15 upgrade #2204

[saschaszott]

related issue: #3226

[atarix83]

Regarding this topic I'd propose to integrate specific parts of the ng-dynamic-forms library into the DSpace base code. Specifically, we should port the core and ui-ng-bootstrap modules.

While @artlowel recently suggested migrating to ngx-formly, it’s important to note that ngx-formly is still based on Angular 13. In contrast, the current version of ng-dynamic-forms is built on Angular 16 and utilizes standalone components, which is advantageous for bundle size. Although ng-dynamic-forms requires updates to the latest versions of Angular and Bootstrap/ng-bootstrap, we (4science) have already initiated this process with a pull request.

By incorporating the necessary parts of ng-dynamic-forms into the base code, we can minimize the workload associated with migrating to a different library. This approach will also simplify code maintenance and version management.

This work could be done in two different phases:

Phase 1: Initial Integration, in order to move the necessary parts of ng-dynamic-forms into the DSpace base code.

• Identify and extract the required modules from ng-dynamic-forms (core and ui-ng-bootstrap).
• Update component references in the existing codebase to point to the newly integrated modules.
• Remove the ng-dynamic-forms dependency from the project.

Estimated Time: 2 days

Phase 2: Refactor the DSpace code that currently uses a custom extension of ui-ng-bootstrap https://github.com/DSpace/dspace-angular/tree/main/src/app/shared/form/builder/ds-dynamic-form-ui.

• Review the custom extension of ui-ng-bootstrap in the DSpace code (ds-dynamic-form-ui).
• Refactor the code to align with the integrated ng-dynamic-forms modules.
• Ensure all custom functionalities are preserved and optimized.
• Conduct thorough testing to verify the refactored code works as expected.

Estimated Time: 4 days.

[artlowel]

While @artlowel recently suggested migrating to ngx-formly, it’s important to note that ngx-formly is still based on Angular 13.

The minimal required version of angular is version 13, but it is compatible with angular 17, as you can tell from my proof of concept. It builds without peer dependency warnings. It is likely also compatible with 18, but I haven't verified that myself yet. Its dependencies were last updated last week, so I'm sure if there were breaking changes they'd make a dedicated version for angular 18

By incorporating the necessary parts of ng-dynamic-forms into the base code, we can minimize the workload associated with migrating to a different library. This approach will also simplify code maintenance and version management.

There are very few DSpace developers that understand how to even use the ng-dynamic-forms library well enough to make meaningful changes to the submission. If we were to include it into our codebase they'd have to learn to maintain the lib itself too. That's just not something that is likely to happen.

It would also be a big PR that we couldn't really review properly, but would just have to merge and hope for the best.

We should take this opportunity to move away from that lib, and rewrite the submission into something that is easier to understand, modify and maintain.

It doesn't have to be formly, as discussed, I'm also working on a PoC using Angular 18 forms, but I'm convinced sticking with ng-dynamic-forms is a bad idea

[atarix83]

The minimal required version of angular is version 13, but it is compatible with angular 17, as you can tell from my proof of concept. It builds without peer dependency warnings. It is likely also compatible with 18, but I haven't verified that myself yet. Its dependencies were last updated last week, so I'm sure if there were breaking changes they'd make a dedicated version for angular 18

However, it’s not just about compatibility. It still relies on the old modules approach, which isn’t optimized for the standalone approach (see ngx-formly/ngx-formly#3721 (comment))

There are very few DSpace developers that understand how to even use the ng-dynamic-forms library well enough to make meaningful changes to the submission. If we were to include it into our codebase they'd have to learn to maintain the lib itself too. That's just not something that is likely to happen.

In my opinion, replacing it with another library would present the same challenge.
Even if we start from scratch, the new code would still need to be maintained, and other developers would have to learn it to ensure proper maintenance.

The main difference with my proposal is the workload. Both replacing the library and starting from scratch would require a significant amount of time, around 100-150 hours, or even more if we consider fixing bugs and addressing accessibility issues we’ve already resolved.

By including the library in our codebase, we would have a working solution that could serve as a solid foundation for improvements, especially considering the new features available in Angular 18, such as the unified control state change events combined with Signals.

[artlowel]

Even if we start from scratch, the new code would still need to be maintained, and other developers would have to learn it to ensure proper maintenance

But we could design it to be much easier to understand and much easier to maintain. It currently isn't, and bringing in that entire lib is only going to exacerbate that problem.

Of course it would be less work right now. But we'd be pushing the problem of the refactor we've known has been necessary for years down the road yet again, taking on a bunch of extra code to maintain in the process. By going for the quick fix now, the submission will remain hard to modify, debug and maintain, which will end up costing us a lot more effort in the long run.

By including the library in our codebase, we would have a working solution that could serve as a solid foundation for improvements, especially considering the new features available in Angular 18, such as the unified control state change events combined with Signals

This lib is only going to get support for angular 18 and things like "unified control state change events combined with signals" if we add it ourselves. So we might as well do that without that lib in the first place, or get it in the next version of formly