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

(feat) Unify session types #387

Merged
merged 1 commit into from
Apr 8, 2022
Merged

(feat) Unify session types #387

merged 1 commit into from
Apr 8, 2022

Conversation

ibacher
Copy link
Member

@ibacher ibacher commented Apr 7, 2022

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.

For changes to apps

If applicable

  • My work includes tests or is validated by existing tests.
  • I have updated the esm-framework mock to reflect any API changes I have made.

Summary

This is an larger change than I had intended, but the point is to unify the ways we are using the session and user objects in core. While I've collapsed a couple of interfaces, these are not (as far as I can see) used outside of the framework itself, so this is technically not a breaking change.

I've also renamed useSessionUser to useSession as this more accurately reflects what it returns (the user data is a property of the session), but for backwards compatibility, I kept the useSessionUser alias around. At a later point, we might want to change things so useSession returns the session and useSessionUser just returns the user.

Screenshots

Related Issue

Other

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

File size impact

Merging unify_session_handling into master impact files as follow:

@openmrs/esm-devtools-app (-0.02%)
Files new size
packages/apps/esm-devtools-app/dist/614.js 413 kB (-79 B / -0.02%) ↘️
Unmodified (4) 25.6 kB (0 B / +0%) 👻
Total (5) 439 kB (-79 B / -0.02%) ↘️
@openmrs/esm-implementer-tools-app (-0%)
Files new size
packages/apps/esm-implementer-tools-app/dist/614.js 413 kB (-79 B / -0.02%) ↘️
Unmodified (20) 1.94 MB (0 B / +0%) 👻
Total (21) 2.35 MB (-79 B / -0%) ↘️
@openmrs/esm-login-app (-0%)
Files new size
packages/apps/esm-login-app/dist/614.js 413 kB (-79 B / -0.02%) ↘️
Unmodified (27) 1.62 MB (0 B / +0%) 👻
Total (28) 2.03 MB (-79 B / -0%) ↘️
@openmrs/esm-offline-tools-app (-0.01%)
Files new size
packages/apps/esm-offline-tools-app/dist/614.js 413 kB (-79 B / -0.02%) ↘️
packages/apps/esm-offline-tools-app/dist/574.js 2.13 kB (-127 B / -5.64%) ↘️
Unmodified (24) 2.37 MB (0 B / +0%) 👻
Total (26) 2.78 MB (-206 B / -0.01%) ↘️
@openmrs/esm-primary-navigation-app (-0%)
Files new size
packages/apps/esm-primary-navigation-app/dist/614.js 413 kB (-79 B / -0.02%) ↘️
Unmodified (17) 1.78 MB (0 B / +0%) 👻
Total (18) 2.19 MB (-79 B / -0%) ↘️
@openmrs/esm-app-shell (+0%)
Files new size
packages/shell/esm-app-shell/dist/openmrs.js 1.42 MB (+42 B / +0%) ↗️
packages/shell/esm-app-shell/dist/service-worker.js 164 kB (0 B / +0%) 👻
Unmodified (2) 537 kB (0 B / +0%) 👻
Total (4) 2.12 MB (+42 B / +0%) ↗️
Generated by @jsenv/file-size-impact during Report bundle size#2115422743 on 012fdee

@ibacher ibacher merged commit d25cae1 into master Apr 8, 2022
@ibacher ibacher deleted the unify_session_handling branch April 8, 2022 13:51
@ZacButko ZacButko mentioned this pull request May 3, 2022
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.

2 participants