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

Normalized jquery bundle #6970

Merged
merged 2 commits into from Nov 6, 2018

Conversation

Projects
None yet
3 participants
@Nerdinacan
Copy link
Contributor

commented Nov 6, 2018

So we KIND OF had a solution for this. Previously we jammed all jquery plugins into a script that frequently gets included (onload.js) in entry points that are rendered by webpack. However, that can fail because of webpack's tree-shaking: explicitly when you render an entry point that does not require onload.js.

This PR modifies the webpack configs to guarantee the same version of jquery with the same attached plugins regardless of whether that particular onload.js script got included. This was something that could occasionally happen in iframe documents.

This is accomplished pretty easily with webpack's module aliasing rules. The "jQuery" module now points to a file called "jquery.custom.js" which builds on "jqueryVendor" which points at the npm or local lib version. Then we use the expose-loader to put the jquery variable on every page rendered by webpack regardless of whether or not jquery was included in the source script.

The end result is the same version of jquery with the same plugins everywhere in the site. So no more weird ".select2 is not a function" errors.

@galaxybot galaxybot added the triage label Nov 6, 2018

@galaxybot galaxybot added this to the 19.01 milestone Nov 6, 2018

@Nerdinacan

This comment has been minimized.

Copy link
Contributor Author

commented Nov 6, 2018

Found a general bug in one of the selenium tests. Don't think it's related to this specific PR, but I can address it.

test_import_dataset_from_history
This test fails if there's more than one saved history in the dropdown. It looks like the test always assumed that there would be only one saved history on the dropdown, so it didn't bother selecting the one it cared about, but now there's an "API history" or something in there, probably from some other test.

@dannon

This comment has been minimized.

Copy link
Member

commented Nov 6, 2018

(to document it here, I think it's the test from https://github.com/galaxyproject/galaxy/pull/6729/files#diff-aaafd90514e4845df6eaed99c3ce8be1R67 that's likely having a side effect in the recent selenium test failures, causing this problem)

@dannon

This comment has been minimized.

Copy link
Member

commented Nov 6, 2018

To clarify a little, this isn't addressing a bug in the current dev codebase. It breaks all our jquery plugins out of onload, which we won't want to include in all entrypoints necessarily moving forward (though we do now).

We're going to follow up on the selenium fix (which is unrelated to this) in a subsequent PR.

@dannon dannon merged commit eb57469 into galaxyproject:dev Nov 6, 2018

5 of 6 checks passed

selenium test Build finished. 148 tests run, 2 skipped, 1 failed.
Details
api test Build finished. 439 tests run, 1 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 190 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 268 tests run, 10 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details

@Nerdinacan Nerdinacan deleted the Nerdinacan:jquery_bundle branch Nov 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.