Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: migrate to standalone components, directives, pipes #2370

Closed
enea4science opened this issue Jul 15, 2023 · 2 comments · Fixed by #2750
Closed

feat: migrate to standalone components, directives, pipes #2370

enea4science opened this issue Jul 15, 2023 · 2 comments · Fixed by #2750
Assignees
Labels
claimed: 4Science 4Science team is working on this issue & will contribute back code task new feature
Milestone

Comments

@enea4science
Copy link
Contributor

Is your feature request related to a problem? Please describe.
No. But having big SharedModule is an anti-pattern and slows down compilation times.

Describe the solution you'd like
Having standalone components, directives and pipes makes it easier to think about the dependencies of the components and are easier to refactor.

The official standalone migration schematic does 99% of the work:

ng generate @angular/core:standalone

There are some places where we need to use the forwardRef for components that are used before initialisation, but those were easy fixable.

I did a test, and the migration worked fine, the project compiles fine and didn't have any thing broken from a quick look I had.

Not all tests pass (even though they get updated by the schematic), but most of them fail because of providers, and those are easily fixable imo.

Additional context
Here are screenshots from what I tried:

image image
@enea4science enea4science added needs triage New issue needs triage and/or scheduling new feature labels Jul 15, 2023
@github-project-automation github-project-automation bot moved this to 🆕 Triage in DSpace Backlog Jul 15, 2023
@enea4science
Copy link
Contributor Author

Also, there would be no need for LazyThemeModule as standalone fixes the context issue.

image

@tdonohue
Copy link
Member

@enea4science : Thanks for this... I like this idea overall, but I suspect it needs discussion from others. I've linked this ticket into our Developers Meeting agenda for this week, as it sounds semi-related the the Angular refactor proposal that @atarix83 will talk about. This ticket seems to me to suggest a first step towards cleaning up our existing Angular components. https://wiki.lyrasis.org/display/DSPACE/2023-07-20+DSpace+Developers+Meeting#id-20230720DSpaceDevelopersMeeting-Agenda

@tdonohue tdonohue added code task and removed needs triage New issue needs triage and/or scheduling labels Sep 11, 2023
@tdonohue tdonohue moved this from 📋 To Do to 🏗 In Progress in DSpace 8.0 Release Dec 14, 2023
@tdonohue tdonohue added the claimed: 4Science 4Science team is working on this issue & will contribute back label Jan 18, 2024
@tdonohue tdonohue added this to the 8.0 milestone Mar 8, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In Progress to ✅ Done in DSpace 8.0 Release Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
claimed: 4Science 4Science team is working on this issue & will contribute back code task new feature
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants