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) Allow useConfig() to load configuration from other modules #751

Merged
merged 2 commits into from
Aug 18, 2023

Conversation

ibacher
Copy link
Member

@ibacher ibacher commented Aug 16, 2023

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.

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

Occasionally, we want a module to be able to load the configuration from a different module; this is possible in the underlying configuration API, but not via the useConfig() hook. This PR just extends the useConfig() hook to allow you to pass an optional module name, in which case useConfig() will return the configuration of that module. Note that the usage of this feature is discouraged as it may add unnecessary coupling between modules.

Screenshots

Related Issue

Other

@github-actions
Copy link
Contributor

github-actions bot commented Aug 16, 2023

Size Change: +372 B (0%)

Total Size: 2.13 MB

Filename Size Change
packages/shell/esm-app-shell/dist/openmrs.f663f41be72dd2b5.js 0 B -211 kB (removed) 🏆
packages/shell/esm-app-shell/dist/openmrs.30f0548f5c5d4622.js 211 kB +211 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size Change
packages/apps/esm-devtools-app/dist/68.js 4.29 kB 0 B
packages/apps/esm-devtools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-devtools-app/dist/889.js 109 kB +36 B (0%)
packages/apps/esm-devtools-app/dist/988.js 282 B 0 B
packages/apps/esm-devtools-app/dist/main.js 2.95 kB 0 B
packages/apps/esm-devtools-app/dist/openmrs-esm-devtools-app.js 3.03 kB 0 B
packages/apps/esm-implementer-tools-app/dist/102.js 2.76 kB 0 B
packages/apps/esm-implementer-tools-app/dist/139.js 11.7 kB 0 B
packages/apps/esm-implementer-tools-app/dist/199.js 28.4 kB 0 B
packages/apps/esm-implementer-tools-app/dist/242.js 6.71 kB 0 B
packages/apps/esm-implementer-tools-app/dist/246.js 447 B 0 B
packages/apps/esm-implementer-tools-app/dist/319.js 501 B 0 B
packages/apps/esm-implementer-tools-app/dist/369.js 2.63 kB 0 B
packages/apps/esm-implementer-tools-app/dist/426.js 1.66 kB 0 B
packages/apps/esm-implementer-tools-app/dist/466.js 5.82 kB 0 B
packages/apps/esm-implementer-tools-app/dist/499.js 2.77 kB 0 B
packages/apps/esm-implementer-tools-app/dist/500.js 2.83 kB 0 B
packages/apps/esm-implementer-tools-app/dist/574.js 470 B 0 B
packages/apps/esm-implementer-tools-app/dist/587.js 4.18 kB 0 B
packages/apps/esm-implementer-tools-app/dist/600.js 62.2 kB 0 B
packages/apps/esm-implementer-tools-app/dist/620.js 126 kB 0 B
packages/apps/esm-implementer-tools-app/dist/625.js 472 B 0 B
packages/apps/esm-implementer-tools-app/dist/657.js 472 B 0 B
packages/apps/esm-implementer-tools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-implementer-tools-app/dist/757.js 470 B 0 B
packages/apps/esm-implementer-tools-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-implementer-tools-app/dist/807.js 1.44 kB 0 B
packages/apps/esm-implementer-tools-app/dist/833.js 580 B 0 B
packages/apps/esm-implementer-tools-app/dist/889.js 109 kB +41 B (0%)
packages/apps/esm-implementer-tools-app/dist/895.js 2.27 kB 0 B
packages/apps/esm-implementer-tools-app/dist/main.js 4.12 kB 0 B
packages/apps/esm-implementer-tools-app/dist/openmrs-esm-implementer-tools-app.js 3.25 kB 0 B
packages/apps/esm-login-app/dist/236.js 272 B 0 B
packages/apps/esm-login-app/dist/240.js 364 B 0 B
packages/apps/esm-login-app/dist/261.js 6.25 kB 0 B
packages/apps/esm-login-app/dist/263.js 6.65 kB 0 B
packages/apps/esm-login-app/dist/272.js 264 B 0 B
packages/apps/esm-login-app/dist/319.js 480 B 0 B
packages/apps/esm-login-app/dist/336.js 234 B 0 B
packages/apps/esm-login-app/dist/337.js 1.3 kB 0 B
packages/apps/esm-login-app/dist/390.js 603 B 0 B
packages/apps/esm-login-app/dist/438.js 6 kB 0 B
packages/apps/esm-login-app/dist/539.js 298 B 0 B
packages/apps/esm-login-app/dist/574.js 428 B 0 B
packages/apps/esm-login-app/dist/610.js 5.8 kB 0 B
packages/apps/esm-login-app/dist/611.js 726 B 0 B
packages/apps/esm-login-app/dist/625.js 421 B 0 B
packages/apps/esm-login-app/dist/63.js 16.5 kB 0 B
packages/apps/esm-login-app/dist/642.js 10.7 kB 0 B
packages/apps/esm-login-app/dist/644.js 305 B 0 B
packages/apps/esm-login-app/dist/673.js 284 B 0 B
packages/apps/esm-login-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-login-app/dist/737.js 24.9 kB 0 B
packages/apps/esm-login-app/dist/757.js 509 B 0 B
packages/apps/esm-login-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-login-app/dist/807.js 685 B 0 B
packages/apps/esm-login-app/dist/829.js 10.1 kB 0 B
packages/apps/esm-login-app/dist/833.js 528 B 0 B
packages/apps/esm-login-app/dist/889.js 109 kB +40 B (0%)
packages/apps/esm-login-app/dist/main.js 3.48 kB 0 B
packages/apps/esm-login-app/dist/openmrs-esm-login-app.js 3.17 kB 0 B
packages/apps/esm-offline-tools-app/dist/202.js 9.67 kB 0 B
packages/apps/esm-offline-tools-app/dist/206.js 7.37 kB 0 B
packages/apps/esm-offline-tools-app/dist/263.js 6.66 kB 0 B
packages/apps/esm-offline-tools-app/dist/281.js 12.3 kB 0 B
packages/apps/esm-offline-tools-app/dist/290.js 438 B 0 B
packages/apps/esm-offline-tools-app/dist/316.js 2.06 kB 0 B
packages/apps/esm-offline-tools-app/dist/319.js 1.04 kB 0 B
packages/apps/esm-offline-tools-app/dist/391.js 305 B 0 B
packages/apps/esm-offline-tools-app/dist/412.js 1.58 kB 0 B
packages/apps/esm-offline-tools-app/dist/47.js 6.31 kB 0 B
packages/apps/esm-offline-tools-app/dist/501.js 23.4 kB 0 B
packages/apps/esm-offline-tools-app/dist/526.js 15.6 kB 0 B
packages/apps/esm-offline-tools-app/dist/538.js 6.75 kB 0 B
packages/apps/esm-offline-tools-app/dist/541.js 384 B 0 B
packages/apps/esm-offline-tools-app/dist/574.js 1.04 kB 0 B
packages/apps/esm-offline-tools-app/dist/594.js 11.2 kB 0 B
packages/apps/esm-offline-tools-app/dist/625.js 1.04 kB 0 B
packages/apps/esm-offline-tools-app/dist/63.js 16.5 kB 0 B
packages/apps/esm-offline-tools-app/dist/697.js 5.47 kB 0 B
packages/apps/esm-offline-tools-app/dist/733.js 290 B 0 B
packages/apps/esm-offline-tools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-offline-tools-app/dist/752.js 2.83 kB 0 B
packages/apps/esm-offline-tools-app/dist/757.js 1.21 kB 0 B
packages/apps/esm-offline-tools-app/dist/78.js 6.13 kB 0 B
packages/apps/esm-offline-tools-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-offline-tools-app/dist/807.js 1.03 kB 0 B
packages/apps/esm-offline-tools-app/dist/82.js 51.8 kB 0 B
packages/apps/esm-offline-tools-app/dist/833.js 1.23 kB 0 B
packages/apps/esm-offline-tools-app/dist/889.js 109 kB +40 B (0%)
packages/apps/esm-offline-tools-app/dist/937.js 6.56 kB 0 B
packages/apps/esm-offline-tools-app/dist/967.js 492 B 0 B
packages/apps/esm-offline-tools-app/dist/main.js 59.4 kB 0 B
packages/apps/esm-offline-tools-app/dist/openmrs-esm-offline-tools-app.js 3.25 kB 0 B
packages/apps/esm-primary-navigation-app/dist/113.js 11.6 kB 0 B
packages/apps/esm-primary-navigation-app/dist/241.js 13.8 kB -2 B (0%)
packages/apps/esm-primary-navigation-app/dist/263.js 6.66 kB 0 B
packages/apps/esm-primary-navigation-app/dist/319.js 183 B 0 B
packages/apps/esm-primary-navigation-app/dist/414.js 467 B 0 B
packages/apps/esm-primary-navigation-app/dist/574.js 183 B 0 B
packages/apps/esm-primary-navigation-app/dist/625.js 184 B 0 B
packages/apps/esm-primary-navigation-app/dist/63.js 16.5 kB 0 B
packages/apps/esm-primary-navigation-app/dist/631.js 1.07 kB 0 B
packages/apps/esm-primary-navigation-app/dist/710.js 6.44 kB 0 B
packages/apps/esm-primary-navigation-app/dist/716.js 6.08 kB 0 B
packages/apps/esm-primary-navigation-app/dist/735.js 2.64 kB 0 B
packages/apps/esm-primary-navigation-app/dist/743.js 7.17 kB 0 B
packages/apps/esm-primary-navigation-app/dist/757.js 209 B 0 B
packages/apps/esm-primary-navigation-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-primary-navigation-app/dist/807.js 182 B 0 B
packages/apps/esm-primary-navigation-app/dist/833.js 210 B 0 B
packages/apps/esm-primary-navigation-app/dist/889.js 109 kB +41 B (0%)
packages/apps/esm-primary-navigation-app/dist/948.js 3.03 kB 0 B
packages/apps/esm-primary-navigation-app/dist/main.js 4.78 kB 0 B
packages/apps/esm-primary-navigation-app/dist/openmrs-esm-primary-navigation-app.js 3.29 kB 0 B
packages/framework/esm-api/dist/openmrs-esm-api.js 9.68 kB 0 B
packages/framework/esm-breadcrumbs/dist/openmrs-esm-breadcrumbs.js 2.66 kB 0 B
packages/framework/esm-config/dist/openmrs-esm-module-config.js 7.87 kB 0 B
packages/framework/esm-dynamic-loading/dist/openmrs-esm-dynamic-loading.js 2.12 kB 0 B
packages/framework/esm-error-handling/dist/openmrs-esm-error-handling.js 894 B 0 B
packages/framework/esm-extensions/dist/openmrs-esm-extensions.js 8.02 kB 0 B
packages/framework/esm-feature-flags/dist/openmrs-esm-feature-flags.js 1.67 kB 0 B
packages/framework/esm-framework/dist/530.openmrs-esm-framework.js 2.92 kB 0 B
packages/framework/esm-framework/dist/645.openmrs-esm-framework.js 9.31 kB 0 B
packages/framework/esm-framework/dist/710.openmrs-esm-framework.js 6.48 kB 0 B
packages/framework/esm-framework/dist/735.openmrs-esm-framework.js 2.66 kB 0 B
packages/framework/esm-framework/dist/788.openmrs-esm-framework.js 42.9 kB 0 B
packages/framework/esm-framework/dist/925.openmrs-esm-framework.js 6.69 kB 0 B
packages/framework/esm-framework/dist/openmrs-esm-framework.js 171 kB +91 B (0%)
packages/framework/esm-globals/dist/openmrs-esm-globals.js 714 B 0 B
packages/framework/esm-offline/dist/openmrs-esm-offline.js 34.4 kB 0 B
packages/framework/esm-react-utils/dist/openmrs-esm-react-utils.js 18.5 kB +37 B (0%)
packages/framework/esm-state/dist/openmrs-esm-state.js 888 B 0 B
packages/framework/esm-styleguide/dist/openmrs-esm-styleguide.js 18.4 kB 0 B
packages/framework/esm-utils/dist/openmrs-esm-utils.js 10.2 kB 0 B
packages/shell/esm-app-shell/dist/44878c380ebabdae.js 6.01 kB 0 B
packages/shell/esm-app-shell/dist/9894b1727d6bd536.js 1.58 kB 0 B
packages/shell/esm-app-shell/dist/e861b352d93d75e6.js 9.88 kB 0 B
packages/shell/esm-app-shell/dist/service-worker.js 57.3 kB +1 B (0%)
packages/tooling/openmrs/dist/cli.js 3.08 kB 0 B
packages/tooling/openmrs/dist/commands/assemble.js 2.73 kB 0 B
packages/tooling/openmrs/dist/commands/build.js 1.61 kB 0 B
packages/tooling/openmrs/dist/commands/debug.js 579 B 0 B
packages/tooling/openmrs/dist/commands/develop.js 1.84 kB 0 B
packages/tooling/openmrs/dist/commands/index.js 438 B 0 B
packages/tooling/openmrs/dist/commands/start.js 848 B 0 B
packages/tooling/openmrs/dist/index.js 517 B 0 B
packages/tooling/openmrs/dist/runner.js 870 B 0 B
packages/tooling/openmrs/dist/utils/config.js 726 B 0 B
packages/tooling/openmrs/dist/utils/debugger.js 529 B 0 B
packages/tooling/openmrs/dist/utils/dependencies.js 639 B 0 B
packages/tooling/openmrs/dist/utils/helpers.js 393 B 0 B
packages/tooling/openmrs/dist/utils/importmap.js 3.25 kB 0 B
packages/tooling/openmrs/dist/utils/index.js 444 B 0 B
packages/tooling/openmrs/dist/utils/logger.js 234 B 0 B
packages/tooling/openmrs/dist/utils/npmConfig.js 854 B 0 B
packages/tooling/openmrs/dist/utils/untar.js 721 B 0 B
packages/tooling/openmrs/dist/utils/variables.js 192 B 0 B
packages/tooling/openmrs/dist/utils/webpack.js 276 B 0 B
packages/tooling/webpack-config/dist/index.js 3.24 kB 0 B

compressed-size-action

T = Omit<ConfigObject, "Display conditions" | "Translation overrides">
>() {
export function useConfig<T = Record<string, any>>(
moduleName: string | null | undefined = undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not

Suggested change
moduleName: string | null | undefined = undefined
moduleName?: string

?

@@ -160,26 +160,28 @@ function useNormalConfig(moduleName: string) {
/**
* Use this React Hook to obtain your module's configuration.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest renaming moduleName to something like foreignModuleName and adding a docstring comment explaining that it is not needed for normal use and indeed should not be used if it can be avoided. Making useConfig take an options object as its argument so that users have to type out foreignModuleName or whatever in order to pass one might be even better.

Copy link
Collaborator

@brandones brandones left a comment

Choose a reason for hiding this comment

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

Thanks Ian! Nice test, too.

@brandones brandones merged commit dabecc7 into main Aug 18, 2023
6 checks passed
@brandones brandones deleted the feat/use-config-other-modules branch August 18, 2023 14:38
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