Skip to content

isTrustedSignalshould return false when no trusted set has been defined#5500

Merged
jhefferman-sfdc merged 3 commits intomasterfrom
jhefferman/trusted-signals-fix
Sep 17, 2025
Merged

isTrustedSignalshould return false when no trusted set has been defined#5500
jhefferman-sfdc merged 3 commits intomasterfrom
jhefferman/trusted-signals-fix

Conversation

@jhefferman-sfdc
Copy link
Contributor

Details

An internal Signals API is not being called in this context, as state managers are not in use. When the internal API is not called, all objects are considered signals and a warning is logged for properties that do not meet the criteria.

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.

GUS work item

W-19640386

@jhefferman-sfdc jhefferman-sfdc requested a review from a team as a code owner September 17, 2025 18:16
{
"path": "packages/@lwc/engine-dom/dist/index.js",
"maxSize": "25.0KB"
"maxSize": "24.73KB"
Copy link
Contributor

Choose a reason for hiding this comment

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

That's 0.01KB more than your last PR! 📉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sigh, "corrected"


import Test from 'x/test';
import { Signal } from 'x/signal';
import { spyConsole } from '../../../helpers/console.js';
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO spyConsole doesn't add any value over just using vitest's spyOn. I want to remove spyConsole eventually, but that's a problem for later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tech debt ftw

@jhefferman-sfdc jhefferman-sfdc merged commit 82a6410 into master Sep 17, 2025
7 checks passed
@jhefferman-sfdc jhefferman-sfdc deleted the jhefferman/trusted-signals-fix branch September 17, 2025 20:36
Copy link
Member

@ekashida ekashida left a comment

Choose a reason for hiding this comment

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

LGTM! The bug being fixed looked obviously wrong so that was throwing me off, but it's actually the result of multiple changes made over time. Thanks for this explanation @wjhsf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants