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.

Sign up for GitHub

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

Jump to bottom

feat(core): Make the isStandalone() function available in public API #48114

Closed
wants to merge 1 commit into from
Closed

feat(core): Make the isStandalone() function available in public API #48114

wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Nov 18, 2022 •
edited

Making isStandalone() a public api

fixes #47919

PR Type

What kind of change does this PR introduce?

  • Feature

Does this PR introduce a breaking change?

  • No (outside of removing ɵisStandalone)

Other information

It's the first time I'm exposing a previously private API. Feedbacks are welcome !

@pullapprove pullapprove bot requested review from alxhub and atscott November 18, 2022 09:19
@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Nov 18, 2022
@JeanMeche JeanMeche force-pushed the feature/public-istandalone-47919 branch 2 times, most recently from 7b51738 to c16e64b Compare November 18, 2022 12:46
@alan-agius4 alan-agius4 added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Nov 18, 2022
AndrewKushnir
AndrewKushnir reviewed Nov 18, 2022
View reviewed changes
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment •
edited

Choose a reason for hiding this comment

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

@JeanMeche thanks for the PR 👍

I've left some comments, but also wanted to propose an update to the error message:

feat(core): Make the `isStandalone()` function available in public API

This commit updates an internal `isStandalone` function and exposes it as a public API,
so that it can be used in applications code.

Fixes #48114.

Thank you.

packages/core/src/render3/definition.ts Outdated Show resolved Hide resolved
packages/core/test/render3/definition_spec.ts Outdated Show resolved Hide resolved
packages/core/test/render3/definition_spec.ts Outdated Show resolved Hide resolved
packages/core/test/render3/definition_spec.ts Outdated Show resolved Hide resolved
@AndrewKushnir AndrewKushnir added area: core Issues related to the framework runtime cross-cutting: standalone Issues related to the NgModule-less world labels Nov 18, 2022
@ngbot ngbot bot added this to the Backlog milestone Nov 18, 2022
@JeanMeche JeanMeche force-pushed the feature/public-istandalone-47919 branch 15 times, most recently from 247778b to c5dcac7 Compare November 21, 2022 09:43
@JeanMeche
Copy link
Member Author

@AndrewKushnir I took your suggestions into account !

AndrewKushnir
AndrewKushnir reviewed Nov 21, 2022
View reviewed changes
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.

@JeanMeche thanks for addressing the feedback! I've added a few more comments.

packages/core/src/render3/definition.ts Outdated Show resolved Hide resolved
packages/core/src/render3/jit/module.ts Outdated Show resolved Hide resolved
packages/core/test/acceptance/standalone_spec.ts Outdated Show resolved Hide resolved
packages/core/test/acceptance/standalone_spec.ts Outdated Show resolved Hide resolved
packages/core/test/acceptance/standalone_spec.ts Outdated Show resolved Hide resolved
packages/core/test/acceptance/standalone_spec.ts Outdated Show resolved Hide resolved
packages/core/src/render3/definition.ts Outdated Show resolved Hide resolved
packages/core/src/render3/definition.ts Outdated Show resolved Hide resolved
packages/core/test/acceptance/standalone_spec.ts Outdated Show resolved Hide resolved
packages/core/test/acceptance/standalone_spec.ts Outdated Show resolved Hide resolved
@JeanMeche JeanMeche force-pushed the feature/public-istandalone-47919 branch 2 times, most recently from 24d63cd to e1cc34f Compare November 21, 2022 22:03
@JeanMeche JeanMeche force-pushed the feature/public-istandalone-47919 branch 2 times, most recently from 39db4ad to 6cc7a8c Compare November 30, 2022 21:45
AndrewKushnir
AndrewKushnir approved these changes Nov 30, 2022
View reviewed changes
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.

@JeanMeche the changes look great, I'm adding an LGTM. We'd need to get a couple more approvals since this PR changes the public API surface.

packages/core/test/acceptance/standalone_spec.ts Show resolved Hide resolved
jessicajaniuk
jessicajaniuk approved these changes Dec 1, 2022
View reviewed changes
Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment •
edited

Choose a reason for hiding this comment

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

LGTM! This looks great! Thanks, @JeanMeche!

reviewed-for: public-api, fw-core

@pullapprove pullapprove bot requested a review from dylhunn December 1, 2022 17:04
@JeanMeche JeanMeche force-pushed the feature/public-istandalone-47919 branch from 6cc7a8c to bddbd00 Compare December 1, 2022 17:52
@JeanMeche
feat(core): Make the isStandalone() function available in public API
7593c39 
This commit updates an internal `isStandalone` function and exposes it as a public API,
so that it can be used in applications code.

fixes angular#47919
@JeanMeche JeanMeche force-pushed the feature/public-istandalone-47919 branch from bddbd00 to 7593c39 Compare December 1, 2022 18:00
@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 1, 2022
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir
Copy link
Contributor

Caretaker Note: Please ignore the google-internal-tests status. Unrelated and pre-existing failing targets.

@AndrewKushnir AndrewKushnir added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Dec 1, 2022
@AndrewKushnir
Copy link
Contributor

This PR was merged into the repository by commit 3842157.

@AndrewKushnir
Copy link
Contributor

@JeanMeche thanks again for creating this PR and addressing the feedback 👍
The PR is now merged and will be released as a part of Angular v15.1.0 in a few weeks.

sr5434 pushed a commit to sr5434/angular that referenced this pull request Dec 3, 2022
@JeanMeche @sr5434
feat(core): Make the isStandalone() function available in public API (
549cf8b 
angular#48114)

This commit updates an internal `isStandalone` function and exposes it as a public API,
so that it can be used in applications code.

fixes angular#47919

PR Close angular#48114
@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 Jan 1, 2023
trekladyone pushed a commit to trekladyone/angular that referenced this pull request Feb 1, 2023
@JeanMeche @trekladyone
feat(core): Make the isStandalone() function available in public API (
2ab3f46 
angular#48114)

This commit updates an internal `isStandalone` function and exposes it as a public API,
so that it can be used in applications code.

fixes angular#47919

PR Close angular#48114
@JeanMeche JeanMeche deleted the feature/public-istandalone-47919 branch May 2, 2023 12:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

@alxhub alxhub Awaiting requested review from alxhub

@atscott atscott Awaiting requested review from atscott

@pkozlowski-opensource pkozlowski-opensource Awaiting requested review from pkozlowski-opensource

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime cross-cutting: standalone Issues related to the NgModule-less world detected: feature PR contains a feature commit merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: minor This PR is targeted for the next minor release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

Create method to reflect type of Directive and Pipe.
5 participants
@JeanMeche @AndrewKushnir @atscott @jessicajaniuk @alan-agius4