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

fix: add signals as dependency #4106

Merged
merged 2 commits into from Mar 27, 2024
Merged

fix: add signals as dependency #4106

merged 2 commits into from Mar 27, 2024

Conversation

jmsjtu
Copy link
Member

@jmsjtu jmsjtu commented Mar 27, 2024

Details

Not sure why this hasn't broken sooner but the @lwc/signals package is causing breakages in our nucleus down streams.

The reason is because the the SignalTracker imports the Signal type from @lwc/signals and because it hasn't been added as a dependency it causes breakages.

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-15276750

@jmsjtu jmsjtu requested a review from a team as a code owner March 27, 2024 21:47
@jmsjtu
Copy link
Member Author

jmsjtu commented Mar 27, 2024

Removing the Signal export from lwc package for now.

There is additional work in 252 on signals and we will export it at that time.

import { logWarnOnce } from '../../shared/logger';
import type { Signal } from '@lwc/signals';
Copy link
Member Author

Choose a reason for hiding this comment

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

This type needs to be re-exported in SignalTracker as part of the function definition types.

@jmsjtu
Copy link
Member Author

jmsjtu commented Mar 27, 2024

Looks like some downstream are failing for a separate typescript issue, see #4107

@jmsjtu jmsjtu merged commit 8ab5109 into master Mar 27, 2024
9 checks passed
@jmsjtu jmsjtu deleted the jtu/signals-dependency branch March 27, 2024 22:19
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.

None yet

4 participants