(BREAKING) Retire concept of ConnectedExtension #1154
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +189 kB (+3.18%) Total Size: 6.11 MB
ℹ️ View Unchanged
|
brandones
reviewed
Oct 1, 2024
brandones
reviewed
Oct 1, 2024
| @@ -1,5 +1,5 @@ | |||
| import React from 'react'; | |||
| import { act, render, screen, waitFor } from '@testing-library/react'; | |||
| import { act, render, prettyDOM, screen, waitFor } from '@testing-library/react'; | |||
There was a problem hiding this comment.
Suggested change
| import { act, render, prettyDOM, screen, waitFor } from '@testing-library/react'; | |
| import { act, render, screen, waitFor } from '@testing-library/react'; |
Unused?
There was a problem hiding this comment.
Yeah, I was using it for troubleshooting and forgot to remove it.
brandones
reviewed
Oct 1, 2024
| if (openWorkspace.title == getWorkspaceTitle(openWorkspace, openWorkspace.additionalProps)) { | ||
| openWorkspace.title = getWorkspaceTitle(openWorkspace, newWorkspace.additionalProps); | ||
| if (openWorkspace.title === getWorkspaceTitle(openWorkspace, openWorkspace.additionalProps)) { | ||
| openWorkspace.title = getWorkspaceTitle(newWorkspace, newWorkspace.additionalProps); |
ibacher
force-pushed
the
breaking/retire-connected-extensions
branch
from
4651685
to
097b0f6
Compare
denniskigen
added a commit
to openmrs/openmrs-esm-patient-chart
that referenced
this pull request
Oct 8, 2024
…ponent This fix addresses a potential runtime error in the VisitSummary component when a visit has no extensions. The error occurs due to recent changes in the [useConnectedExtensions](openmrs/openmrs-esm-core#1154) hook, which possibly now loads extensions asynchronously. The [current implementation](https://github.com/openmrs/openmrs-esm-patient-chart/blob/3b13b5b5d7ce39dbe54508a2041ececb28f2d33e/packages/esm-patient-chart-app/src/visit/visits-widget/past-visits-components/visit-summary.component.tsx#L157) assumes extensions are always available when rendering. If extensions are not yet loaded when the component renders, it throws an error. I've remedied this by adding optional chaining to the extensions mapping. This ensures the component gracefully handles cases where extensions are undefined or not yet loaded.
Samstar10
pushed a commit
to Samstar10/openmrs-esm-core
that referenced
this pull request
Oct 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
feat,fix, orchore, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
esm-core tracks extensions in various forms (there are others; these are the relevant ones here):
ExtensionDefinition: An extension as defined in an appsroutes.jsonExtensionRegistration: An extension registered with the extension systemAssignedExtension: An extension instance, representing an extension in a specific slotConnectedExtension: An "assigned" extension that is also available due to either the presence of a feature flag or the browsers online status and the extensions definition.This PR collapses the distinction between
AssignedExtensionandConnectedExtension, that is, an extension is only assigned to a slot now if:This is accomplished by making the "output extension store" dependent on the featureFlag store, the browser's online status, and the current session store. These dependencies are now centralized as part of the internal
getAssignedExtensionsFromSlotData()and the relevant public APIs have been updated to supply the necessary data.Prior to this change, assigned extensions were not recalculated globally, but instead, each instance of an
ExtensionSlotwould calluseConnectedExtensions(), which would load the current feature flags and online status and use these to filter theAssignedExtensions.There are two reasons for this change:
useConnectedExtensions()instance.This PR removes all of the
ConnectedExtensioninternals, however, theuseConnectedExtensions()hook (now an alias foruseAssignedExtensions()) remains so as to not break too much code.I've also taken this opportunity to completely remove the support for the
wrapproperty from the ReactExtensionobject.Screenshots
Related Issue
Other