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

Trigger signals and callbacks for messages not present in messages provided by loadMessages plugin #2800

Merged
merged 5 commits into from
May 21, 2024

Conversation

martin-lysk
Copy link
Contributor

@martin-lysk martin-lysk commented May 17, 2024

resolves opral/inlang-message-sdk#67
fixes Reactivity for messages that are deleted
adds a test for deleted and created messages

Copy link

changeset-bot bot commented May 17, 2024

🦋 Changeset detected

Latest commit: 9e0dee6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@inlang/sdk Patch
@inlang/badge Patch
@inlang/cli Patch
@inlang/doc-layout-component Patch
@inlang/editor Patch
@inlang/github-lint-action Patch
vs-code-extension Patch
@inlang/message-bundle-component Patch
@inlang/rpc Patch
@inlang/settings-component Patch
@inlang/telemetry Patch
@inlang/cross-sell-ninja Patch
@inlang/plugin-i18next Patch
@inlang/plugin-json Patch
@inlang/plugin-m-function-matcher Patch
@inlang/plugin-next-intl Patch
@inlang/plugin-t-function-matcher Patch
@inlang/paraglide-unplugin Patch
@inlang/paraglide-js-e2e Patch
@inlang/paraglide-next-e2e Patch
@inlang/sdk-load-test Patch
@inlang/server Patch
next-js-testapp Patch
@inlang/sdk-multi-project-test Patch
@inlang/paraglide-rollup Patch
@inlang/paraglide-vite Patch
@inlang/paraglide-webpack Patch
@inlang/paraglide-astro Patch
@inlang/paraglide-sveltekit Patch
@inlang/paraglide-sveltekit-example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@martin-lysk
Copy link
Contributor Author

@jldec the fix was rather small - we just need to go through the messages loaded already and substract the new messages coming from the plugin. The messages left are those that are deleted.

I remember that we discussed this when we worked on the new persistence - where messages that are not present in the set of imported messages does not mean a message was deleted - it was just not in the source used by the importer.
Until we deprecate loadMessagesPlugins in favor of importer this assumption doesn't hold and its clearly a bug.

I also added a test for message creation that was missing as well.

@martin-lysk martin-lysk marked this pull request as ready for review May 17, 2024 22:51
Copy link
Contributor

@jldec jldec 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 one thing needed in the test.

Copy link
Member

@LorisSigrist LorisSigrist left a comment

Choose a reason for hiding this comment

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

Thanks!

@martin-lysk
Copy link
Contributor Author

I just found a bug in fsWatch cleanup - fixed it and also added the test to probe for inserts and updates

@martin-lysk martin-lysk merged commit 9567b71 into main May 21, 2024
3 checks passed
@martin-lysk martin-lysk deleted the martinlysk1/mesdk-106 branch May 21, 2024 13:26
@github-actions github-actions bot locked and limited conversation to collaborators May 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

messages.getAll.subscribe changed callback is not triggered when a message is removed
3 participants