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

refactor(server): remove deprecated renderModuleFactory #49247

Closed
wants to merge 1 commit into from

Conversation

alan-agius4
Copy link
Contributor

@alan-agius4 alan-agius4 commented Feb 28, 2023

The deprecated renderModuleFactory has been removed as it is no longer necessary with Ivy.

BREAKING CHANGE: renderModuleFactory has been removed. Use renderModule instead.

//CC @AndrewKushnir

@alan-agius4 alan-agius4 added the area: server Issues related to server-side rendering label Feb 28, 2023
@ngbot ngbot bot added this to the Backlog milestone Feb 28, 2023
@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Feb 28, 2023
The deprecated `renderModuleFactory` has been removed as it is no longer necessary with Ivy.

BREAKING CHANGE: `renderModuleFactory` has been removed. Use `renderModule` instead.
@alan-agius4 alan-agius4 added target: major This PR is targeted for the next major release and removed area: server Issues related to server-side rendering labels Feb 28, 2023
@ngbot ngbot bot modified the milestone: Backlog Feb 28, 2023
@alan-agius4 alan-agius4 added the area: server Issues related to server-side rendering label Feb 28, 2023
@ngbot ngbot bot added this to the Backlog milestone Feb 28, 2023
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Feb 28, 2023
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alan-agius4 thanks for the cleanup 👍

A couple quick comments:

  • We can also update an error message here to exclude "[Factory]".
  • There is one instance where renderModuleFactory is referenced in g3 code which we may need to cleanup before landing this change (just search for renderModuleFactory).

Thank you.

@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Feb 28, 2023
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-for: public-api

@alan-agius4
Copy link
Contributor Author

As discussed on slack, the G3 reference is not a problem as it’s an integration test of a NPM package which is not run in G3.

Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api, fw-platform-server

@jessicajaniuk jessicajaniuk added the action: merge The PR is ready for merge by the caretaker label Feb 28, 2023
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 17abe6d.

@alan-agius4 alan-agius4 deleted the render-module-factory branch March 1, 2023 20:42
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: server Issues related to server-side rendering detected: breaking change PR contains a commit with a breaking change target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants