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 moving the
`mathjs` logic to the chart renderer so that we could continue benefiting from
asynchronous script loading.
----------------------------------------------------------------
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]
