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

Signal queries - runtime bits #54017

Conversation

pkozlowski-opensource
Copy link
Member

No description provided.

@pkozlowski-opensource pkozlowski-opensource added the area: core Issues related to the framework runtime label Jan 22, 2024
@ngbot ngbot bot added this to the Backlog milestone Jan 22, 2024
@pkozlowski-opensource pkozlowski-opensource force-pushed the signal_queries_pr branch 2 times, most recently from 5e2e316 to a99ec1e Compare January 22, 2024 13:46
packages/core/src/render3/query.ts Show resolved Hide resolved
packages/core/src/authoring/queries.ts Outdated Show resolved Hide resolved
packages/core/src/linker/query_list.ts Outdated Show resolved Hide resolved
packages/core/src/linker/query_list.ts Outdated Show resolved Hide resolved
packages/core/src/render3/query_reactive.ts Outdated Show resolved Hide resolved
packages/core/src/render3/query_reactive.ts Show resolved Hide resolved
})();

export function bindQueryToSignal(target: Signal<unknown>, queryIndex: number): void {
const node = target[SIGNAL] as QuerySignalNode<unknown>;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Also may want to assert here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, will do in the follow-up!

@pkozlowski-opensource pkozlowski-opensource marked this pull request as ready for review January 22, 2024 14:16
@pkozlowski-opensource pkozlowski-opensource 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 Jan 22, 2024
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM (just follow-up tasks)

function createQuerySignalFn<V>(firstOnly: true, required: true): Signal<V>;
function createQuerySignalFn<V>(firstOnly: true, required: false): Signal<V|undefined>;
function createQuerySignalFn<V>(firstOnly: false, required: false): Signal<ReadonlyArray<V>>;
function createQuerySignalFn<V>(firstOnly: boolean, required: boolean) {
Copy link
Member

Choose a reason for hiding this comment

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

(For follow-up) nit: Would be good to include JSDocs here as well

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, let's do in the follow-up

@pkozlowski-opensource pkozlowski-opensource force-pushed the signal_queries_pr branch 3 times, most recently from 90efad9 to d2b752c Compare January 23, 2024 17:07
packages/core/src/linker/query_list.ts Outdated Show resolved Hide resolved
packages/core/src/render3/query_reactive.ts Show resolved Hide resolved
packages/core/src/render3/query_reactive.ts Outdated Show resolved Hide resolved
This is a refactor commit that moves more query construction / refresh
logic from the body of instructions into dedicated functions. This is
in preparation for the signal-based query instructions.
This refactoring expands the QueryList such that we can add onDirty
callback to be invoked when a given query gets marked as dirty during
view insertion / removal. This mechanism is needed for signal-based
queries.
This commit adds new instructions to support view queries as signals.
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

Copy link
Member

@alxhub alxhub 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

@pkozlowski-opensource pkozlowski-opensource added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 24, 2024
@ngbot
Copy link

ngbot bot commented Jan 24, 2024

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google-internal-tests" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@pkozlowski-opensource pkozlowski-opensource added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jan 24, 2024
@pkozlowski-opensource
Copy link
Member Author

jessicajaniuk pushed a commit that referenced this pull request Jan 24, 2024
This refactoring expands the QueryList such that we can add onDirty
callback to be invoked when a given query gets marked as dirty during
view insertion / removal. This mechanism is needed for signal-based
queries.

PR Close #54017
jessicajaniuk pushed a commit that referenced this pull request Jan 24, 2024
…54017)

This commit adds new instructions to support view queries as signals.

PR Close #54017
@jessicajaniuk
Copy link
Contributor

This PR was merged into the repository by commit c043128.

atscott pushed a commit to atscott/angular that referenced this pull request Jan 26, 2024
…4017)

This is a refactor commit that moves more query construction / refresh
logic from the body of instructions into dedicated functions. This is
in preparation for the signal-based query instructions.

PR Close angular#54017
atscott pushed a commit to atscott/angular that referenced this pull request Jan 26, 2024
…54017)

This refactoring expands the QueryList such that we can add onDirty
callback to be invoked when a given query gets marked as dirty during
view insertion / removal. This mechanism is needed for signal-based
queries.

PR Close angular#54017
atscott pushed a commit to atscott/angular that referenced this pull request Jan 26, 2024
…ngular#54017)

This commit adds new instructions to support view queries as signals.

PR Close angular#54017
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…4017)

This is a refactor commit that moves more query construction / refresh
logic from the body of instructions into dedicated functions. This is
in preparation for the signal-based query instructions.

PR Close angular#54017
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…54017)

This refactoring expands the QueryList such that we can add onDirty
callback to be invoked when a given query gets marked as dirty during
view insertion / removal. This mechanism is needed for signal-based
queries.

PR Close angular#54017
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…ngular#54017)

This commit adds new instructions to support view queries as signals.

PR Close angular#54017
danieljancar pushed a commit to danieljancar/angular that referenced this pull request Jan 26, 2024
…4017)

This is a refactor commit that moves more query construction / refresh
logic from the body of instructions into dedicated functions. This is
in preparation for the signal-based query instructions.

PR Close angular#54017
danieljancar pushed a commit to danieljancar/angular that referenced this pull request Jan 26, 2024
…54017)

This refactoring expands the QueryList such that we can add onDirty
callback to be invoked when a given query gets marked as dirty during
view insertion / removal. This mechanism is needed for signal-based
queries.

PR Close angular#54017
danieljancar pushed a commit to danieljancar/angular that referenced this pull request Jan 26, 2024
…ngular#54017)

This commit adds new instructions to support view queries as signals.

PR Close angular#54017
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…4017)

This is a refactor commit that moves more query construction / refresh
logic from the body of instructions into dedicated functions. This is
in preparation for the signal-based query instructions.

PR Close angular#54017
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…54017)

This refactoring expands the QueryList such that we can add onDirty
callback to be invoked when a given query gets marked as dirty during
view insertion / removal. This mechanism is needed for signal-based
queries.

PR Close angular#54017
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…ngular#54017)

This commit adds new instructions to support view queries as signals.

PR Close angular#54017
nikvarma pushed a commit to nikvarma/angular that referenced this pull request Jan 31, 2024
…4017)

This is a refactor commit that moves more query construction / refresh
logic from the body of instructions into dedicated functions. This is
in preparation for the signal-based query instructions.

PR Close angular#54017
nikvarma pushed a commit to nikvarma/angular that referenced this pull request Jan 31, 2024
…54017)

This refactoring expands the QueryList such that we can add onDirty
callback to be invoked when a given query gets marked as dirty during
view insertion / removal. This mechanism is needed for signal-based
queries.

PR Close angular#54017
nikvarma pushed a commit to nikvarma/angular that referenced this pull request Jan 31, 2024
…ngular#54017)

This commit adds new instructions to support view queries as signals.

PR Close angular#54017
@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 Mar 5, 2024
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: core Issues related to the framework runtime 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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants