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

Register to globally available version of jQuery #360

Merged
merged 2 commits into from
Jun 11, 2018
Merged

Register to globally available version of jQuery #360

merged 2 commits into from
Jun 11, 2018

Conversation

abhishekdev
Copy link
Contributor

I am not entirely sure about the maintainers preferences here, but it seems importing the plugin in a ES6 environment has no effect.

Issue:

The plugin does not register on window.jquery OR to the locally available jQuery in the module.

Expectation

  • The plugin should register itself to the global available jQuery or the local ES6 import

Sample code

Contents of main.js

import jQuery from './jquery-shim';
import 'jquery-ui-dist/jquery-ui';
import 'jquery-mousewheel';
import 'jscrollpane/script/jquery.jscrollpane';

Contents of /jquery-shim.js

import jQuery from 'jquery';
// expose jQuery on window, so that non ES6 legacy plugins (incl. jquery-ui) may work
window.jQuery = window.$ = jQuery;

export default jQuery;

The pull requests gives preference to the globally available jquery (if present) before using the private jquery dependency as a fallback

@illuusio
Copy link
Collaborator

Thank you for contribution! How does this work on ES5 which is current normal?

@abhishekdev
Copy link
Contributor Author

I had not verified this before. However, it seems the plugin does not work in CommonJS code as well. It works on the ES5 browser environment as it registers on the globally available jQuery object.

CommonJS code:

var jquery = require('jquery')
var jscrollpane = require("jscrollpane")

console.log(jquery.fn.jscrollpane, jscrollpane);

Run Testcase: https://runkit.com/embed/5vp5rhyarvj7

@illuusio
Copy link
Collaborator

illuusio commented Jun 7, 2018

Can you update to new release

- Support plugin in ES6 modules
@abhishekdev
Copy link
Contributor Author

abhishekdev commented Jun 7, 2018

Sure! Updated to latest master.

Also here is an updated testcase: https://runkit.com/embed/6bn0ostlytwz.

PS: If everything looks fine, I can update the version number in package.json

@illuusio
Copy link
Collaborator

illuusio commented Jun 7, 2018

One more thing as this is 'big' change I think we'll follow RC path so could you add this to rc1. Nobody can say I break their favorite tool.
I'll update package.json as I update lock file, thanks for offer

@abhishekdev
Copy link
Contributor Author

So would you update the version numbers (to include RC suffix) in the changelog as well?

Just wanted to make sure my understanding is correct and there is nothing more on my plate.

Thanks again for looking into this.

@illuusio
Copy link
Collaborator

illuusio commented Jun 8, 2018

Actually It would be easiest to remove Changelog or make it 2.2.1-rc.1 if you like to keep it. I'll make release after this as I can this make somebody unhappy.

- Changelog would be updated on merge
@abhishekdev
Copy link
Contributor Author

Removed changelog.

@illuusio illuusio merged commit d752d57 into vitch:master Jun 11, 2018
@illuusio
Copy link
Collaborator

Merged thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants