feat: lazily load plugin scripts and dependencies when needed (DHIS2-10518) #1546

amcgee · 2021-02-17T23:44:55Z

Remove blocking external javascript script tags from index.html head
Dynamically load legacy plugin scripts when they are needed, not before
Dynamically import() the Data Visualizer plugin so that it can be code-split out of the main bundle. (NOTE: for this to really be useful it requires a tree-shakable analytics library, which we don't have yet)

This also removes the need for a custom index.html and for setting any environment variables in development

DV and Maps plugins will hopefully move to dynamic modules, but this solution will still be required for eventReports and eventVisualizer

Here is a production example : https://debug.dhis2.org/2.35dev/api/apps/dashboard2/index.html#/Goz4vyRx2cy

NOTE: ER and EV plugins don't work locally because of CORS

TODO:

Decide whether to use prefetch, preload, or neither for data-visualizer-plugin chunk (currently prefetching)
Investigate EV EXT error
Show <LoadingMask> while fetching scripts
Remove console.logs

jenniferarnesen · 2021-02-18T12:40:33Z

only map - loads map.js and dv plugin. Dashboard without map - no map.js
no visualizations should not load any plugins. Actual - loads dv plugin, but not the other plugins
only ev - loads eventchart.js and dv plugin chunk
only er - loads eventreport.js and dv plugin chunk
only chart/pivot - loads dv plugin chunk and no other plugins

I'm getting a bunch of errors on the production instance resulting in ev not rendering:

3UiManager.js?d38e:536 Uncaught ReferenceError: Ext is not defined
    at m.n.alert (UiManager.js?d38e:536)
    at Object.<anonymous> (Layout.js?bf56:459)
    at u (jquery-3.3.1.min.js:2)
    at Object.fireWith [as rejectWith] (jquery-3.3.1.min.js:2)
    at k (jquery-3.3.1.min.js:2)
    at XMLHttpRequest.<anonymous> (jquery-3.3.1.min.js:2)

Dashboard with only a text item

Dashboard with a chart and pivot table

amcgee · 2021-02-18T12:59:27Z

@jenniferarnesen the duplication of data-visualizer-plugin chunk there is expected - the javascript type is from a <link> element with rel="prefetch" inject into the html by webpack to prefetch and cache the script as early as possible, but not block rendering. That way it's immediately available if it's needed later. I'd like to investigate whether we want to switch to preload instead... but I don't think this shouold be detrimental here.

prefetch | Specifies that the browser should preemptively fetch and cache the target resource as it is likely to be required for a follow-up navigation
preload | Specifies that the browser agent must preemptively fetch and cache the target resource for current navigation according to the destination given by the "as" attribute (and the priority associated with that destination).

amcgee · 2021-02-18T13:03:06Z

Strange that we're seeing the EV EXT error now though.... i am seeing it too but it was working for me before. Will have to investigate that.

amcgee · 2021-02-18T13:17:54Z

Re: the EV EXT error

This error also appears on the current dashboard https://debug.dhis2.org/2.35dev/dhis-web-dashboard/#/PZIxO0YLxFt -- the eventchart.js plugin tries to use EXT to show an alert saying "the user admin doesn't have access to this program", but there's no EXT available (that's only in the app itself). It works with the system user, which has access to all programs. So this appears to be a bug in the EV plugin and/or we need to also load EXT when loading that plugin (which can be done but... ew)

amcgee · 2021-03-11T00:41:43Z

@jenniferarnesen I've updated this a bit (better late than never) to add loading mask and remove console logs, the implementation changed a bit to support this. Think we can get this in to 2.36 before the hard freeze?

jenniferarnesen · 2021-03-11T07:45:41Z

src/components/Item/VisualizationItem/Visualization/Visualization.js

+                            onLoadingComplete={this.onLoadingComplete}
+                            forDashboard={true}
+                            style={pluginProps.style}
+                            userSettings={{
+                                displayProperty: this.context.userSettings
+                                    .keyAnalysisDisplayProperty,
+                            }}


forDashboard, userSettings and onLoadingComplete could be moved to DataVisualizerPlugin. Should be pretty straightforward (onLoadingComplete just sets a state var that is only relevant for DataVisualizerPlugin). But if you don't have time for this, I can do it. If its not done by noon (either by me or you), then let's just go ahead and merge as is, and I can do this later.

Changes committed

amcgee · 2021-03-11T11:28:23Z

@jenniferarnesen nice, much cleaner thanks! Changes look good, shall we merge? 👍

# [31.14.0](v31.13.10...v31.14.0) (2021-03-11) ### Features * lazily load plugin scripts and dependencies when needed (DHIS2-10518) ([#1546](#1546)) ([c13eafe](c13eafe))

dhis2-bot · 2021-03-11T11:54:28Z

🎉 This PR is included in version 31.14.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

amcgee added 3 commits February 18, 2021 00:26

feat: dynamically load plugin scripts and dependencies when needed

0cf8fae

chore: fix some tests

758ca98

fix: wait for in-flight fetches of the same plugin type

d8cc844

Merge branch 'master' into feat/lazy-load-plugins

28511f1

Merge branch 'master' into feat/lazy-load-plugins

2297f5c

jenniferarnesen approved these changes Feb 18, 2021

View reviewed changes

amcgee added 2 commits March 9, 2021 00:06

Merge remote-tracking branch 'origin/master' into feat/lazy-load-plugins

94ced91

Merge branch 'master' into feat/lazy-load-plugins

164226b

amcgee marked this pull request as ready for review March 11, 2021 00:40

jenniferarnesen reviewed Mar 11, 2021

View reviewed changes

jenniferarnesen approved these changes Mar 11, 2021

View reviewed changes

jenniferarnesen added 3 commits March 11, 2021 10:49

fix: move visualizer props to DataVisualizerPlugin

2a723fb

fix: remove unused state

21bb3c3

fix: change var name to avoid confusion between plugin and vis loading

401d3f0

amcgee merged commit c13eafe into master Mar 11, 2021

amcgee deleted the feat/lazy-load-plugins branch March 11, 2021 11:41

dhis2-bot added a commit that referenced this pull request Mar 11, 2021

chore(release): cut 31.14.0 [skip ci]

fd9e19b

# [31.14.0](v31.13.10...v31.14.0) (2021-03-11) ### Features * lazily load plugin scripts and dependencies when needed (DHIS2-10518) ([#1546](#1546)) ([c13eafe](c13eafe))

dhis2-bot added the released label Mar 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: lazily load plugin scripts and dependencies when needed (DHIS2-10518) #1546

feat: lazily load plugin scripts and dependencies when needed (DHIS2-10518) #1546

amcgee commented Feb 17, 2021 •

edited

Loading

jenniferarnesen commented Feb 18, 2021 •

edited

Loading

amcgee commented Feb 18, 2021

amcgee commented Feb 18, 2021

amcgee commented Feb 18, 2021

amcgee commented Mar 11, 2021 •

edited

Loading

jenniferarnesen Mar 11, 2021

jenniferarnesen Mar 11, 2021

amcgee commented Mar 11, 2021

dhis2-bot commented Mar 11, 2021

feat: lazily load plugin scripts and dependencies when needed (DHIS2-10518) #1546

feat: lazily load plugin scripts and dependencies when needed (DHIS2-10518) #1546

Conversation

amcgee commented Feb 17, 2021 • edited Loading

jenniferarnesen commented Feb 18, 2021 • edited Loading

amcgee commented Feb 18, 2021

amcgee commented Feb 18, 2021

amcgee commented Feb 18, 2021

amcgee commented Mar 11, 2021 • edited Loading

jenniferarnesen Mar 11, 2021

Choose a reason for hiding this comment

jenniferarnesen Mar 11, 2021

Choose a reason for hiding this comment

amcgee commented Mar 11, 2021

dhis2-bot commented Mar 11, 2021

amcgee commented Feb 17, 2021 •

edited

Loading

jenniferarnesen commented Feb 18, 2021 •

edited

Loading

amcgee commented Mar 11, 2021 •

edited

Loading