ktmud commented on a change in pull request #11199:
URL:
https://github.com/apache/incubator-superset/pull/11199#discussion_r521887850
##########
File path:
superset-frontend/cypress-base/cypress/integration/explore/AdhocFilters.test.ts
##########
@@ -23,11 +23,11 @@ describe('AdhocFilters', () => {
cy.route('GET', '/superset/explore_json/**').as('getJson');
cy.route('POST', '/superset/explore_json/**').as('postJson');
cy.route('GET', '/superset/filter/table/*/name').as('filterValues');
- });
-
- it('Should not load mathjs when not needed', () => {
cy.visitChartByName('Boys'); // a table chart
cy.verifySliceSuccess({ waitAlias: '@postJson' });
+ });
+
+ xit('Should not load mathjs when not needed', () => {
Review comment:
The test is failing because of this import chain:
`@superset-ui/plugin-chart-echarts -> Timeseries -> transformProps ->
utils/annotation -> mathjs`. Unlike `loadChart`, `transformProps` is not loaded
asynchronously, so Webpack could not defer the loading of `mathjs`, which means
it will be loaded for all first-visits on almost all pages, undoing part of the
optimization we did in #10837 .
The test is there to make sure this optimization still works. It didn't
break because it was flaky. It caught the breaking changes it is supposed to
catch.
It's OK to merge this PR as is for now, but I'd recommend finding ways to
move the `mathjs` logic to the chart renderer so that we could continue
benefiting from code split and asynchronous loading.
In general I think `transformProps` should be as lightweight as possible and
preferably only return scalars. It should only be about applying defaults and
consolidating parameter names/shapes. Most complex logics should be dealt with
either by the chart renderer itself or at another data-processing layer on top
of the chart. This way it's easier to reuse your chart at other places outside
of Superset charting plugins.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
