Restore dynamic component decorators by alexandrevryghem · Pull Request #4528 · DSpace/dspace-angular

alexandrevryghem · 2025-07-03T13:14:10Z

References

Fixes Size regression in main.js bundle caused by standalone component migration #2899

Description

When DSpace was migrated to standalone components the decorators on all the dynamic components were removed. The way dynamic components used to work was by mapping the different types of supported components with their component declaration. These mappings were automatically populated by the decorators themselves once the class was loaded, so that's why we always loaded those components eagerly. During the switch to standalone components the logic to populate the maps automatically was replaced with hardcoded maps. This PR now generates these hardcoded decorator maps automatically during every rebuild using the new npm run generate:decorator:registries script, and we also lazy loaded all the component imports to reduce the main bundle size.

Instructions for Reviewers

List of changes in this PR:

A new script scripts/generate-decorator-registries.ts has been created that goes over the whole code and finds all the decorators defined inside the DECORATORS array. These decorators are then all bundled together in a registry file that will be used to retrieve the matching component.
A new custom webpack plugin has been created to run npm run generate:decorator:registries on every rebuild
All the existing decorators that were not themable yet, have been made themable
Update the listable-object.decorator.ts#getMatch not always being able to fall back to a default component
Used the listable-object.decorator.ts#getMatch logic in all the decorators to find the best match

Guidance for how to test this PR:

Go over all the decorators listed in generate-decorator-registries.ts#DECORATORS and verify that they are still all rendering correctly in prod & dev mode
Verify the bundle size is now smaller (757.6KB → 631.27KB)

Checklist

…orator maps Co-authored-by: Nona Luypaert <nona.luypaert@atmire.com> Co-authored-by: Alexandre Vryghem <alexandre@atmire.com>

Co-authored-by: Alexandre Vryghem <alexandre@atmire.com>

…xplicitly

…hods, remove duplicate logic

FrancescoMolinaro

Hello @alexandrevryghem , @artlowel ,

Many thanks for these changes. I’ve taken an initial look at the PR and performed some testing, and I wanted to share some feedback with you.

Overall, I believe this PR is an improvement over the previous static map approach, both in terms of bundle size and maintainability. It also brings back the decorator functionality, which is definitely a nice feature to have.

During my testing, however, I noticed that the script is triggered multiple times during the build process. This could be related to how files are currently being watched by the Webpack plugin, as the script appears to fire on any type of file change. This behavior could lead to unnecessary resource usage and longer build times, especially in development mode.

Additionally, the way the decorators are currently generated raises some concerns regarding the upcoming migration to Nx. As it stands, this kind of independent build process would be difficult to align with Nx’s principles—particularly the project isolation model—and might even break it.

I think it would be best to tailor this solution within the context of an Nx workspace, rethinking how decorators are built. For example, we could isolate each decorator in its own library and ensure the script rebuilds only the files within those libraries when changes occur.

In any case, I believe it would be valuable to discuss this in one of the upcoming community calls, so we can gather feedback from other developers as well.

github-actions · 2025-07-30T22:00:29Z

Hi @alexandrevryghem,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

alexandrevryghem · 2025-09-05T09:52:41Z

@FrancescoMolinaro Thnx for the review & sorry for the delayed response.

I tested the new NX PR, alongside this script and verified that both PRs are compatible. The only thing we would need to change is that the current implementation only searches for decorators in the src folder, but now we would need to include the modules folder as well.

Regarding the isolation of the decorators in the modules, I would not recommend doing that. This would mean that if we wanted to create a new plugin for dspace that adds a @dataservice for example, that we would always have to create the service in the core module. Whereas the idea is that such a plugin should have its own core module that depends the dspace core module.

To prevent services from disabled modules to be available at runtime, we could update the script in the future though to only scan for decorators inside enabled modules.

Regarding performance, the script takes less than half a second to generate all the registries so I don't think this is a problem. If people would however want to disable this auto rebuild it would be easy though, you would only have to remove the plugin from webpack and run npm run generate:decorator:registries manually. But running the script manually takes more time and would complicate the build process for developers so I would leave it enabled by default.

tdonohue · 2026-02-05T14:58:13Z

@alexandrevryghem : When you have a chance, I'd appreciate it if you can rebase this on latest main. Now that #4446 is merged, it seems like this can provide more improvements to themes in 10.x.

I realize there was some discussion above around overlap with Nx (with @FrancescoMolinaro above). But, since the Nx work isn't likely to occur for 10.x, I'd like us all to consider whether this PR is still a useful step in the right direction for 10.0.

…imports

github-actions · 2026-03-13T17:36:12Z

Hi @alexandrevryghem,
Conflicts have been detected against the base branch.
Please resolve these conflicts as soon as you can. Thanks!

FrancescoMolinaro · 2026-04-14T10:31:47Z

Hi @alexandrevryghem , sorry for the delay on this and thanks for your works, I tried this again today and the functionality seems working as expected, the only thing is that gitHub is now signaling 260 files changed and seems that includes a lot of files/changes already on main.
I am not sure if there was an issue in merging the main branch or is just gitHub being "stuck", as the files I have checked look identical.
Could you please have a look so that is clear which files changed in this PR? Maybe just triggering a new comparison from gitHub will be enough to fix it.

alexandrevryghem · 2026-04-14T11:07:30Z

@FrancescoMolinaro thnx for the review! I took a quick look at the current diff on GitHub and all the changes that I can see on my end do look like intentional modifications that are not present on main yet. Could you maybe give some examples of files that shouldn't be included in the diff but are present in yours?

NOTE: we started the development of this right after dspace 9.0 got released so you can look at the original branch this PR started from here, and it has 255 file changes. So having 260 files changes after the merge with main does seem like a reasonable amount of changes

FrancescoMolinaro · 2026-04-14T11:29:43Z

Hi @alexandrevryghem , sorry I think it was an issue only on my side, I was seeing code changes to the yml files for docker and the pipelines, cleaning the browser cache did solve the issue and now I can see the correct files, I thought the number of files changed was high because of that but looking at the correct code changes it is a reasonable number as this PR thouches all the components that previously had a decorator.

I am going to do a code review asap, thanks and sorry again for the false alarm.

FrancescoMolinaro

Hi @alexandrevryghem , thanks again for your work on this, I did check again the code and generally it looks ok, I have found just a couple of things that I would like to address, you can find an inline comment for each of those.
Otherwise is looking good for me, thanks again for the effort.

FrancescoMolinaro · 2026-04-14T13:08:41Z

 import { MenuItemType } from '../menu-item-type.model';
 import { MenuSection } from '../menu-section.model';

+export interface MenuSectionDTO {


Hi @alexandrevryghem , this interface seems to be defined and exported with the same name but different structure in menu.component.ts, if both are needed could we change the name of one of the two so that the difference is clear?

FrancescoMolinaro · 2026-04-14T13:17:19Z

   */
  ngOnInit(): void {
-    this.instantiateComponent();
+    void this.instantiateComponent();


@alexandrevryghem I have seen that this method is now not a void method anymore, could we remove the void operator and add a catch for any possible error from the returned promise? I think otherwise errors in the component instantiation could be swallowed by the void operator resulting just in an empty component.

FrancescoMolinaro · 2026-04-14T13:24:24Z

@@ -0,0 +1,413 @@
+import {


@alexandrevryghem since this file is the core logic of the decorator plugin and it's logic is quite complex, would be possible to add some unit tests so that it would be easier to find issues on future code changes? I think some tests to enforce the functions logic should be enough.

alexandrevryghem self-assigned this Jul 3, 2025

alexandrevryghem added bug performance / caching Related to performance, caching or embedded objects labels Jul 3, 2025

github-advanced-security AI found potential problems Jul 3, 2025

View reviewed changes

Comment thread scripts/generate-decorator-registries.ts Fixed

Comment thread scripts/generate-decorator-registries.ts Fixed

artlowel and others added 2 commits July 3, 2025 16:14

130715: Created generate-decorator-registries script to generator dec…

58a460f

…orator maps Co-authored-by: Nona Luypaert <nona.luypaert@atmire.com> Co-authored-by: Alexandre Vryghem <alexandre@atmire.com>

130715: Restored decorators

db9fa05

Co-authored-by: Alexandre Vryghem <alexandre@atmire.com>

alexandrevryghem force-pushed the restore-decorator-support_contribute-main branch from 5c64f85 to 7952510 Compare July 3, 2025 14:17

tdonohue requested a review from atarix83 July 3, 2025 14:22

tdonohue added this to DSpace 10.0 Release Jul 3, 2025

tdonohue moved this to 🙋 Needs Reviewers Assigned in DSpace 10.0 Release Jul 3, 2025

tdonohue moved this from 🙋 Needs Reviewers Assigned to 👀 Under Review in DSpace 10.0 Release Jul 3, 2025

tdonohue added this to the 10.0 milestone Jul 3, 2025

tdonohue self-requested a review July 3, 2025 14:22

alexandrevryghem and others added 2 commits July 6, 2025 23:20

130715: Fixed fallback values not being used when undefined is used e…

670497a

…xplicitly

130715: Refactor generate-decorator-registries to better organize met…

0e06eee

…hods, remove duplicate logic

FrancescoMolinaro reviewed Jul 17, 2025

View reviewed changes

github-actions Bot added the merge conflict label Jul 30, 2025

Don't skip decorators without components

b043a27

alexandrevryghem mentioned this pull request Nov 27, 2025

Feature/make log in password component themeable #4879

Closed

135912: Add support for numeric arguments in decorators

35dd379

alexandrevryghem mentioned this pull request Feb 4, 2026

Removed theme modules #4446

Merged

12 tasks

Merge branch 'restore-decorator-support_contribute-9.0'

e404968

alexandrevryghem force-pushed the restore-decorator-support_contribute-main branch from 5aa0d62 to 8de9848 Compare February 28, 2026 17:35

github-actions Bot removed the merge conflict label Feb 28, 2026

130715: Made generate:decorator:registries compatible with new @DSpace …

0f4fce3

…imports

alexandrevryghem force-pushed the restore-decorator-support_contribute-main branch from 8de9848 to 0f4fce3 Compare March 1, 2026 12:14

github-actions Bot added the merge conflict label Mar 13, 2026

Merge branch 'main' into restore-decorator-support_contribute-main

d83c030

github-actions Bot removed the merge conflict label Apr 2, 2026

FrancescoMolinaro reviewed Apr 14, 2026

View reviewed changes

Conversation

alexandrevryghem commented Jul 3, 2025 • edited by tdonohue Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

References

Description

Instructions for Reviewers

Checklist

Uh oh!

Uh oh!

Uh oh!

FrancescoMolinaro left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 30, 2025

Uh oh!

alexandrevryghem commented Sep 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdonohue commented Feb 5, 2026

Uh oh!

github-actions Bot commented Mar 13, 2026

Uh oh!

FrancescoMolinaro commented Apr 14, 2026

Uh oh!

alexandrevryghem commented Apr 14, 2026

Uh oh!

FrancescoMolinaro commented Apr 14, 2026

Uh oh!

FrancescoMolinaro left a comment

Choose a reason for hiding this comment

Uh oh!

FrancescoMolinaro Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

FrancescoMolinaro Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

FrancescoMolinaro Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alexandrevryghem commented Jul 3, 2025 •

edited by tdonohue

Loading

FrancescoMolinaro left a comment •

edited

Loading

alexandrevryghem commented Sep 5, 2025 •

edited

Loading