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

Isolate library dependencies #25

Closed
ajmacdonald opened this issue Dec 15, 2017 · 25 comments
Closed

Isolate library dependencies #25

ajmacdonald opened this issue Dec 15, 2017 · 25 comments
Assignees

Comments

@ajmacdonald
Copy link
Collaborator

Make sure jQuery (and other libraries) don't pollute the global scope.

@ajmacdonald ajmacdonald self-assigned this Dec 15, 2017
@jchartrand
Copy link
Collaborator

Is your plan to do something with this:

https://api.jquery.com/jquery.noconflict/

and potentially rewrite all references to $ in the cwrcwriter code to some other name like jqueryCwrc?

@ajmacdonald
Copy link
Collaborator Author

That's a potential solution. However, an additional complication is that there are libraries, e.g. bootstrap, that expect jQuery to be available as window.jQuery or window.$. So I'd need to modify the bootstrap build process.

@jchartrand
Copy link
Collaborator

ah, right - good point. What other options are you considering?

@ajmacdonald
Copy link
Collaborator Author

I'm hoping browserify-shim can be of some use but honestly this a pretty confounding problem to me.

@jchartrand
Copy link
Collaborator

From: https://community.canvaslms.com/thread/19727-jquery-and-bootstrap-conflict

"For anyone else looking for this solution, you basically have to add $.noConflict(true) at the end of your bootstrap.js file. That way Bootstrap will use the updated version of jQuery, and then release it so that Canvas can use the older version of jQuery."

Even if it works, it might not be ideal because we'd have to modify the bootstrap file directly, and it would only help with the bootstrap dependency (and not other libraries that depend on jquery), but maybe we could wrap the imports to all our dependencies somehow, and just release the jquery global at the end of our imports? I guess I still don't see how that would help if a library subsequently tries to get at jquery though global.$ or window.$ or window.jquery. However, maybe the libraries that depend on jquery all obtain and store a reference to jquery when loaded, and then use that subsequently, rather than looking for the global. wdyt?

@jchartrand
Copy link
Collaborator

jchartrand commented Dec 15, 2017

Hmm, looking more closely at that guy's answer (again: https://community.canvaslms.com/thread/19727-jquery-and-bootstrap-conflict) it sounds like he uses browserify to generate a standalone javascript file (probably using the standalone switch https://github.com/browserify/browserify-handbook#standalone). Maybe that somehow, in combination with $.noConflict(true), helps isolate the jquery library to the packaged standalone.

It occurs to me, too, that generating a standalone cwrcwriter (using browserify standalone) might help with integration into islandora, since you could then avoid npm altogether and load the cwrcwriter code through a script tag in the html page. (but maybe you are already doing that)

@jchartrand
Copy link
Collaborator

@jchartrand
Copy link
Collaborator

Do you know which libraries, other than bootstrap, we are using that depend on jquery? If we could make a list that might be helpful for sorting out other conflicts with jquery versions.

@ajmacdonald
Copy link
Collaborator Author

These: https://github.com/cwrc/CWRC-WriterBase/tree/master/src/js/lib/jquery/plugins
And also: jquery-ui, jquery-layout, jstree,

@jchartrand
Copy link
Collaborator

More good stuff here about using noConflict, this time with jstree, but also more generally with jquery plugins:

https://stackoverflow.com/questions/33278178/how-to-use-2-versions-of-jquery-efficiently

And, as I guessed earlier, jquery plugins are indeed supposed to grab a reference to the jquery object at load time, and assign that to a to a reference to $ that is local to the plugin:

https://learn.jquery.com/plugins/basic-plugin-creation/#protecting-the-alias-and-adding-scope

all which makes it sound like we might well be able to use noConflict - we basically load our version of jquery, then load up all our stuff (including jstree, bootstrap, etc.), then call noConflict(true) to remove our reference from the global jquery variables, and revert back to the prior references to the old version of jquery.

We'd still of course have to make sure that we have no references to global.$ or window.$ etc. in our own code, and that we either scope $ locally or we rename our jquery, and all our references to it, to some other name, but that shouldn't be too bad since we have control over that.

wdyt?

@ajmacdonald
Copy link
Collaborator Author

We'd need someone (Brad?) to modify the Drupal code since it currently loads all of its js (including jQuery v1) before CWRC-Writer loads.

@jchartrand
Copy link
Collaborator

jchartrand commented Dec 15, 2017

I think it is fine (in the sense that it will work, but not in the sense that it is a good idea given that the jquery docs say not to do it) to have the older version load first. Apparently jquery keeps track of the previously loaded version and reverts back to it when you call noConflict:

From:

https://api.jquery.com/jquery.noconflict/

Load two versions of jQuery (not recommended). Then, restore jQuery's globally scoped variables to the first loaded jQuery.

 <!doctype html><html lang="en"><head>  <meta charset="utf-8">  <title>jQuery.noConflict demo</title>  <script src="https://code.jquery.com/jquery-1.10.2.js"></script></head><body> <div id="log">  <h3>Before $.noConflict(true)</h3></div><script src="https://code.jquery.com/jquery-1.6.2.js"></script> <script>var $log = $( "#log" ); $log.append( "2nd loaded jQuery version ($): " + $.fn.jquery + "<br>" ); // Restore globally scoped jQuery variables to the first version loaded// (the newer version) jq162 = jQuery.noConflict( true ); $log.append( "<h3>After $.noConflict(true)</h3>" );$log.append( "1st loaded jQuery version ($): " + $.fn.jquery + "<br>" );$log.append( "2nd loaded jQuery version (jq162): " + jq162.fn.jquery + "<br>" );</script> </body></html>

@jchartrand
Copy link
Collaborator

And, from the same doc:

Old references of $ are saved during jQuery initialization; noConflict() simply restores them.

If for some reason two versions of jQuery are loaded (which is not recommended), calling $.noConflict( true ) from the second version will return the globally scoped jQuery variables to those of the first version.```

@jchartrand
Copy link
Collaborator

We might still have to use the browserify --standalone switch to generate our bundle, which is then loaded with the script tag, so that we can control when the noConflict is called. How are you using browserify with the islandora version?

@ajmacdonald
Copy link
Collaborator Author

I'm just using it the standard way: https://github.com/cwrc/Islandora-CWRC-Writer/blob/npmbuild/package.json#L35

@jchartrand
Copy link
Collaborator

How do you load the resulting app.js file into the browser? And how does the Islandora code get a reference to the cwrcWriter (if it does at all)?

@ajmacdonald
Copy link
Collaborator Author

@jchartrand
Copy link
Collaborator

jchartrand commented Dec 15, 2017 via email

@ajmacdonald
Copy link
Collaborator Author

Having a bit of success with this https://stackoverflow.com/a/36146810

@ajmacdonald
Copy link
Collaborator Author

I'm not sure the shim allows for standalone solution though. I'll look into it more next week.

@jchartrand
Copy link
Collaborator

jchartrand commented Dec 15, 2017 via email

@jchartrand
Copy link
Collaborator

jchartrand commented Dec 15, 2017 via email

@jchartrand
Copy link
Collaborator

jchartrand commented Dec 16, 2017 via email

@ajmacdonald
Copy link
Collaborator Author

@jchartrand you're right, it won't. I was heading down the wrong path. I've made some good progress on this today though.

@ajmacdonald
Copy link
Collaborator Author

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

No branches or pull requests

2 participants