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) Support for automatically overriding routes and Carbonize devtools #816

Merged
merged 6 commits into from
Dec 12, 2023

Conversation

ibacher
Copy link
Member

@ibacher ibacher commented Nov 17, 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

This is the first in a planned series of commits to build out better dev tooling for handling stuff related to routes.json files, used by Core V5. The goal of this PR is pretty straight-forward: when adding an app to the import-map-overrides, we should load the routes, if any along-side that.

This does several things:

  1. The DevTools button upstream has been changed a little bit to make it more useful, so with active overrides it now looks like this:
    Screenshot 2023-11-17 at 12 48 10 PM
  2. I Carbonized the import-map-overrides. This is somewhat unnecessary, but I'm planning on adding a routes-specific panel to the devtools eventually. This is basically functionally-equivalent to import-map-overrides.
    Screenshot 2023-11-17 at 12 48 03 PM
  3. When adding a import map override, it also adds a routes override. Similar to import map overrides, these are only processed on page refreshes, so refreshing the page is necessary to see the change.
  4. You can also manually override routes by providing an entry in local storage with the key openmrs-routes:<name of module>, e.g., openmrs-routes:@openmrs/esm-patient-chart-app. The value for this can be either a JSON string (so double-quotes) that points to a URL that resolves to a custom routes.json (this is how 3 above is implemented) or a stringified JSON object that conforms to the routes schema. There is currently no UI for this other than your browser devtools.
  5. I added some sanity checks to the code that registers pages and extensions to ensure that we don't accidentally try to register an invalid page or extension object.

Screenshots

Related Issue

Other

Future plans:

  1. Add a tab to display information about the current active routes files and routes overrides; this will also have some capability to edit the routes overrides directly.
  2. Add a tab to show all registered pages and extensions as well as which of those are active on the page.
  3. Semi-related, it's possible (though no one uses this) to specify an inline import map on the CLI when running develop and some other commands. I thought that I wrote the code to also support that for routes, but it doesn't work, so I'm going to fix that up.

Copy link
Contributor

github-actions bot commented Nov 17, 2023

Size Change: -717 kB (-21%) 🎉

Total Size: 2.71 MB

Filename Size Change
packages/framework/esm-framework/dist/openmrs-esm-framework.js 374 kB -378 kB (-50%) 🏆
packages/shell/esm-app-shell/dist/openmrs.acd08df196ec54d7.js 0 B -332 kB (removed) 🏆
ℹ️ View Unchanged
Filename Size Change
packages/apps/esm-devtools-app/dist/630.js 1.75 kB 0 B
packages/apps/esm-devtools-app/dist/68.js 0 B -4.66 kB (removed) 🏆
packages/apps/esm-devtools-app/dist/729.js 6.79 kB 0 B
packages/apps/esm-devtools-app/dist/735.js 2.63 kB +2 B (0%)
packages/apps/esm-devtools-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-devtools-app/dist/875.js 9.76 kB 0 B
packages/apps/esm-devtools-app/dist/889.js 108 kB +700 B (+1%)
packages/apps/esm-devtools-app/dist/961.js 25.3 kB 0 B
packages/apps/esm-devtools-app/dist/988.js 328 B +46 B (+16%) ⚠️
packages/apps/esm-devtools-app/dist/main.js 3.14 kB +185 B (+6%) 🔍
packages/apps/esm-devtools-app/dist/openmrs-esm-devtools-app.js 3.18 kB +150 B (+5%) 🔍
packages/apps/esm-implementer-tools-app/dist/319.js 633 B 0 B
packages/apps/esm-implementer-tools-app/dist/329.js 10 kB 0 B
packages/apps/esm-implementer-tools-app/dist/426.js 1.66 kB 0 B
packages/apps/esm-implementer-tools-app/dist/460.js 735 B 0 B
packages/apps/esm-implementer-tools-app/dist/56.js 3.07 kB 0 B
packages/apps/esm-implementer-tools-app/dist/560.js 9.6 kB 0 B
packages/apps/esm-implementer-tools-app/dist/574.js 560 B 0 B
packages/apps/esm-implementer-tools-app/dist/587.js 2.81 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 562 B 0 B
packages/apps/esm-implementer-tools-app/dist/651.js 6.86 kB 0 B
packages/apps/esm-implementer-tools-app/dist/727.js 32.9 kB 0 B
packages/apps/esm-implementer-tools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-implementer-tools-app/dist/738.js 6.79 kB 0 B
packages/apps/esm-implementer-tools-app/dist/757.js 560 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 559 B 0 B
packages/apps/esm-implementer-tools-app/dist/833.js 681 B 0 B
packages/apps/esm-implementer-tools-app/dist/889.js 108 kB +677 B (+1%)
packages/apps/esm-implementer-tools-app/dist/913.js 61.2 kB 0 B
packages/apps/esm-implementer-tools-app/dist/main.js 73.3 kB 0 B
packages/apps/esm-implementer-tools-app/dist/openmrs-esm-implementer-tools-app.js 3.3 kB 0 B
packages/apps/esm-login-app/dist/111.js 1.22 kB 0 B
packages/apps/esm-login-app/dist/126.js 2.5 kB 0 B
packages/apps/esm-login-app/dist/173.js 1.22 kB 0 B
packages/apps/esm-login-app/dist/224.js 256 B 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/272.js 264 B 0 B
packages/apps/esm-login-app/dist/319.js 692 B 0 B
packages/apps/esm-login-app/dist/336.js 234 B 0 B
packages/apps/esm-login-app/dist/370.js 30.6 kB 0 B
packages/apps/esm-login-app/dist/460.js 782 B 0 B
packages/apps/esm-login-app/dist/539.js 298 B 0 B
packages/apps/esm-login-app/dist/56.js 3.06 kB 0 B
packages/apps/esm-login-app/dist/574.js 609 B 0 B
packages/apps/esm-login-app/dist/625.js 600 B 0 B
packages/apps/esm-login-app/dist/627.js 257 B 0 B
packages/apps/esm-login-app/dist/63.js 16.5 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/729.js 6.78 kB 0 B
packages/apps/esm-login-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-login-app/dist/757.js 698 B 0 B
packages/apps/esm-login-app/dist/788.js 42.9 kB 0 B
packages/apps/esm-login-app/dist/807.js 961 B 0 B
packages/apps/esm-login-app/dist/812.js 22.5 kB 0 B
packages/apps/esm-login-app/dist/833.js 727 B 0 B
packages/apps/esm-login-app/dist/889.js 108 kB +677 B (+1%)
packages/apps/esm-login-app/dist/main.js 57 kB 0 B
packages/apps/esm-login-app/dist/openmrs-esm-login-app.js 3.32 kB 0 B
packages/apps/esm-offline-tools-app/dist/102.js 81.3 kB 0 B
packages/apps/esm-offline-tools-app/dist/193.js 55 kB 0 B
packages/apps/esm-offline-tools-app/dist/319.js 1.14 kB 0 B
packages/apps/esm-offline-tools-app/dist/460.js 1.32 kB 0 B
packages/apps/esm-offline-tools-app/dist/56.js 3.07 kB 0 B
packages/apps/esm-offline-tools-app/dist/574.js 1.04 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/729.js 6.79 kB 0 B
packages/apps/esm-offline-tools-app/dist/735.js 2.63 kB 0 B
packages/apps/esm-offline-tools-app/dist/757.js 1.2 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.1 kB 0 B
packages/apps/esm-offline-tools-app/dist/833.js 1.22 kB 0 B
packages/apps/esm-offline-tools-app/dist/889.js 108 kB +677 B (+1%)
packages/apps/esm-offline-tools-app/dist/main.js 136 kB 0 B
packages/apps/esm-offline-tools-app/dist/openmrs-esm-offline-tools-app.js 3.29 kB 0 B
packages/apps/esm-primary-navigation-app/dist/262.js 13.3 kB 0 B
packages/apps/esm-primary-navigation-app/dist/319.js 200 B 0 B
packages/apps/esm-primary-navigation-app/dist/460.js 217 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/729.js 6.79 kB 0 B
packages/apps/esm-primary-navigation-app/dist/735.js 18.7 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 235 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 108 kB +677 B (+1%)
packages/apps/esm-primary-navigation-app/dist/960.js 2.64 kB 0 B
packages/apps/esm-primary-navigation-app/dist/main.js 33.7 kB 0 B
packages/apps/esm-primary-navigation-app/dist/openmrs-esm-primary-navigation-app.js 3.23 kB 0 B
packages/framework/esm-api/dist/openmrs-esm-api.js 9.75 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.05 kB 0 B
packages/framework/esm-feature-flags/dist/openmrs-esm-feature-flags.js 1.67 kB 0 B
packages/framework/esm-framework/dist/455.openmrs-esm-framework.js 4.57 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/655.openmrs-esm-framework.js 6.83 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-globals/dist/openmrs-esm-globals.js 756 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 15.2 kB 0 B
packages/framework/esm-routes/dist/openmrs-esm-utils.js 1.43 kB 0 B
packages/framework/esm-state/dist/openmrs-esm-state.js 888 B 0 B
packages/framework/esm-styleguide/dist/openmrs-esm-styleguide.js 19.3 kB 0 B
packages/framework/esm-utils/dist/openmrs-esm-utils.js 11 kB +58 B (+1%)
packages/shell/esm-app-shell/dist/0cca64a12dd8e4d8.js 6.45 kB 0 B
packages/shell/esm-app-shell/dist/9c276181fc6599f8.js 0 B -6.03 kB (removed) 🏆
packages/shell/esm-app-shell/dist/b66a463116735f62.js 3.81 kB 0 B
packages/shell/esm-app-shell/dist/d81faa62d5818f52.js 1.58 kB 0 B
packages/shell/esm-app-shell/dist/openmrs.2b1f9e335565dc1d.js 333 kB 0 B
packages/shell/esm-app-shell/dist/service-worker.js 60.5 kB +5 B (0%)
packages/tooling/openmrs/dist/cli.js 2.86 kB 0 B
packages/tooling/openmrs/dist/commands/assemble.js 2.4 kB 0 B
packages/tooling/openmrs/dist/commands/build.js 1.34 kB 0 B
packages/tooling/openmrs/dist/commands/debug.js 545 B 0 B
packages/tooling/openmrs/dist/commands/develop.js 1.86 kB 0 B
packages/tooling/openmrs/dist/commands/index.js 438 B 0 B
packages/tooling/openmrs/dist/commands/start.js 851 B 0 B
packages/tooling/openmrs/dist/index.js 517 B 0 B
packages/tooling/openmrs/dist/runner.js 637 B 0 B
packages/tooling/openmrs/dist/utils/config.js 728 B 0 B
packages/tooling/openmrs/dist/utils/debugger.js 528 B 0 B
packages/tooling/openmrs/dist/utils/dependencies.js 640 B 0 B
packages/tooling/openmrs/dist/utils/helpers.js 395 B 0 B
packages/tooling/openmrs/dist/utils/importmap.js 2.95 kB 0 B
packages/tooling/openmrs/dist/utils/index.js 444 B 0 B
packages/tooling/openmrs/dist/utils/logger.js 368 B 0 B
packages/tooling/openmrs/dist/utils/npmConfig.js 830 B 0 B
packages/tooling/openmrs/dist/utils/untar.js 722 B 0 B
packages/tooling/openmrs/dist/utils/variables.js 192 B 0 B
packages/tooling/openmrs/dist/utils/webpack.js 278 B 0 B
packages/tooling/webpack-config/dist/index.js 3.58 kB 0 B

compressed-size-action

Copy link
Member

@denniskigen denniskigen left a comment

Choose a reason for hiding this comment

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

Nice work, Ian! Code looks good on first reading. I haven't tested locally yet (dunno if I can get it to build with the issues I've been running into lately)

@denniskigen
Copy link
Member

denniskigen commented Dec 4, 2023

@ibacher, it looks like when I add a module override, and then subsequently remove it, the routesOverride key doesn't get removed automatically from localStorage (ignore the federated module error. I only used that module to showcase the issue):

add-and-remove-override.mp4

@ibacher
Copy link
Member Author

ibacher commented Dec 4, 2023

@denniskigen I see... So, clicking on the "Reset all overrides" was, stupidly, not something I added something for (it should do the right thing if you individually disable the override).

@ibacher
Copy link
Member Author

ibacher commented Dec 4, 2023

Hopefully that's fixed now!

Copy link
Member

@denniskigen denniskigen left a comment

Choose a reason for hiding this comment

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

Looks good to me. I've left a few comments.

@denniskigen
Copy link
Member

One nice thing to support that the library supports is the ability to specify a port number and have the tooling fill out the override URL for you automatically:

port-number-in-overrides-panel.mp4

@denniskigen denniskigen force-pushed the feat/routes-in-devtools branch from 17f9378 to 5371a14 Compare December 12, 2023 18:57
@ibacher
Copy link
Member Author

ibacher commented Dec 12, 2023

One nice thing to support that the library supports is the ability to specify a port number and have the tooling fill out the override URL for you automatically:

Yeah, I've found that to be kinda flaky personally, but it's pretty easy to implement. I'll add it in.

@denniskigen denniskigen force-pushed the feat/routes-in-devtools branch from 9750fb4 to ab39f24 Compare December 12, 2023 20:13
@denniskigen
Copy link
Member

One nice thing to support that the library supports is the ability to specify a port number and have the tooling fill out the override URL for you automatically:

Yeah, I've found that to be kinda flaky personally, but it's pretty easy to implement. I'll add it in.

In that case, probably best to leave it out.

@denniskigen denniskigen merged commit 6e2e2f2 into main Dec 12, 2023
9 checks passed
@denniskigen denniskigen deleted the feat/routes-in-devtools branch December 12, 2023 21:13
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