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

Avoiding duplicate downloads #48

Open
naltatis opened this issue Jun 12, 2017 · 19 comments
Open

Avoiding duplicate downloads #48

naltatis opened this issue Jun 12, 2017 · 19 comments

Comments

@naltatis
Copy link

naltatis commented Jun 12, 2017

I have the issue that scripts are loaded multiple times. I would have expected that scripts that have been loaded or are in the middle of loading won't be loaded each time.

loadjs(['libraryA.js', 'usingA.js'], {
  success: function() { /* done loading */ },
  async: false
});

loadjs(['libraryA.js', 'libraryB_usingA.js', 'usingA_B.js'], {
  success: function() { /* done loading */ },
  async: false
});

In the above example libraryA.js gets loaded twice. I've been trying to avoid this issue by using bundleIds and loadjs.ready but this gets complicated quite quickly.

Am I missing something here?

@amorey
Copy link
Member

amorey commented Jun 12, 2017

LoadJS passes requests through to the browser so it will rely on the browser's caching mechanism if you choose to load a file twice.

If you're using async: false then this should work:

loadjs(['libraryA.js', 'usingA.js', 'libraryB_usingA.js', 'usingA_B.js'], {
  success: function() { /* done loading */ },
  async: false
});

@amorey
Copy link
Member

amorey commented Jun 13, 2017

@naltatis Just checking - did using async: false solve your problem?

@naltatis
Copy link
Author

No that's not really the solution I was looking for. I multiple groups of script files. Each group should download in parallel und execute in series as soon as possible.

Making a long chain of all groups works but delays execution.

I'm currently using a workaround where I register a bundle for each file, wait for it (ready) and then request be next one in the group. This solves the duplicate download issue, but only one file of a group is downloading at the time.

In the solution I'm looking for, loadjs has to keep track of script files that are currently in flight and combine listeners if a duplication occurres.

@amorey
Copy link
Member

amorey commented Jun 14, 2017

Using async: false will load the files in parallel and execute them in series. Let me know if that helps or if you still need to define separate bundles.

@porebskk
Copy link

porebskk commented Jul 14, 2017

I tried to replicate the problem @naltatis mentioned, but I reproduce the behaviour.

Html Code:

<html>
    <head>
        <script src="https://cdn.rawgit.com/muicss/loadjs/3.5.0/dist/loadjs.min.js"></script>
    </head>
    <body>
        <script>
                loadjs(['http://localhost/jquery-with-wait-2s.php', 'http://localhost/bower_components/tooltipster/js/jquery.tooltipster.min.js'], {
                    success: function() {
                        console.log('Loaded jquery file, and tooltipster');
                        },
                    async: false
                });
                loadjs(['http://localhost/jquery-with-wait-2s.php'], {
                    success: function() {
                        console.log('Loaded jquery file');
                        },
                    async: false
                });
                loadjs(['http://localhost/jquery-with-wait-2s.php', 'http://localhost/bower_components/bootstrap/js/tab.js'], {
                    success: function() {
                        console.log('Loaded jquery file and bootstrap tab');
                    },
                    async: false
                });
        </script>
        Hello loadjs
    </body>
</html>

The jquery file got only loaded once.
The console output:
image

Running on Win10 with Chrome btw

@naltatis
Copy link
Author

There seems to be different behaviour in different browsers. In my tests on a Mac I have the duplication issue in Firefox. Chrome and Safari work as expected. I've set up your example in a CodePen:
https://codepen.io/naltatis/pen/YQgGmy

When you open it in debug mode you can see the networking:
https://s.codepen.io/naltatis/debug/YQgGmy/ZoMBajpbweJk

bildschirmfoto 2017-07-14 um 12 25 56

For now, I'm using @amorey' recommendation and put all dependencies in a long list, remove the dups and run dem in async: false.

@porebskk
Copy link

porebskk commented Jul 14, 2017

I get the same error using firefox and IE10 on win10. Chrome, Edge, IE11 on win10 are working fine.

@amorey
Copy link
Member

amorey commented Jul 14, 2017

Great! I'm happy to hear the async: false option is working.

With current browser technology there are only two cross-browser ways to load external files in parallel and execute them in series:

  1. <script> tags embedded in the html on pageload
  2. <script> tags added to the DOM dynamically with the async attribute set to false (this is what LoadJS does when you use async: false)

The new <link rel="preload"> spec gives you more control over the download and execution of external scripts which allows you to do more sophisticated bundling and error handling but it's only available in Chrome and Opera.

@groupedsi
Copy link

Hi,
I have the same problem with files being loaded multiple times. I'm using the latest (3.5.4).

Setup:
html file

<script src="loadjs.js"></script>
<script>loadjs("main.js", 'main');</script>

main.js

console.log("main - loaded");
loadjs("test2.js");
loadjs(["test2.js"]);

test2.js

console.log("test2 - loaded");
loadjs("test3.js");

test3.js

console.log("test3 - loaded");

Behaviour:
- I get "test2 file loaded" twice
- I get "test3 file loaded" twice
- When inspecting the DOM, I see <script src="/areas/admin/assets/test2.js" async></script> twice (same goes for test3)

I've tried the same setup with the $script library and the script tag is there only once per filename (and the console.log appear only once)
Thanks

@amorey
Copy link
Member

amorey commented May 24, 2018

LoadJS will fetch a file every time you call loadjs() and it also accepts arguments as single strings or arrays so these two lines are the same:

loadjs("test2.js");  // fetch test2.js
loadjs(["test2.js"]);  // fetch test2.js again

To avoid multiple fetches you can define a bundle when you fetch the file and then use the .ready() to define any subsequent success/error callbacks:

loadjs('test2.js', 'test2', function() {
  // success callback
});

loadjs.ready('test2', function() {
  // success callback
});

@groupedsi
Copy link

Thanks for the quick response.
I know about both calls being the same, I was just trimming down my example and since they are the same I didn't bother to remove the [].

My problem is about the behaviour of having the script tag inserted many times for the same loadjs("filename").
Since it says

We kept the behavior of the library the same[...]

I thought it would behave the same way when handling the same loadjs("sameFilename") in different places.

Maybe we're going about this the wrong way too, but here's what we're doing:
We're trying to simplify a js plugin system where every coder can loadjs("some seldom used plugin", "pluginname") and later in the code loadjs.ready("pluginname", function(){}). Now every coder needs to make sure the js plugins he needs are loaded.

Coder 1 creates a new plugin (plugin-1.js) and he needs "select2" and "seldomUsedPlugin", so he would do something like this:

//load the requirements
loadjs(["select2", "seldomUsedPlugin", "otherPluginsHere"], "plugin-1-requirements");
//he knows that in the main js file, there is a bundle named jquery has been loadjs
loadjs.ready(["plugin-1-requirements", "jquery"], function(){
	//his code magic
});

Coder 2 creates his new plugin (plugin-2.js) and he needs to make sure he has what he needs

//load the requirements some of which may have already been loadjs'ed
loadjs("seldomUsedPlugin", "plugin-2-requirements"); //same partial requirement as plugin-1
loadjs.ready(["plugin-2-requirements", "jquery"], function(){
	//his code magic
});

Now in a new project I could add plugin-1 or plugin-2 or both. Hence the need to load the same file in both places and we don't want to load all the possible requirements in the main js knowing some might not be used.
Again if loadjs adds a <script> tag with the same src many times, this might have some undesired effects.

Thank you

@amorey
Copy link
Member

amorey commented May 24, 2018

I wasn't aware that $script() is idempotent. In any case, I think it's best to think of LoadJS as a lower-level library that can be used to implement a higher-level dependency manager that fits your specifications. For example, this custom require() method might work for you:

// define bundles here
var bundles = {
  'bundle1': ['file1', 'file2'],
  'bundle2': ['file3', 'file4']
};

function require(bundleIds, callbackFn) {
  bundleIds.forEach(function(bundleId) {
    if (!loadjs.isDefined(bundleId)) loadjs(bundles[bundleId], bundleId);
  });
  loadjs.ready(bundleIds, callbackFn);
}

require(['bundle1'], function() { /* bundle1 lazy-loaded */ });
require(['bundle2'], function() { /* bundle2 lazy-loaded */ });
require(['bundle1', 'bundle2'], function() { /* bundle1 and bundle2 lazy-loaded */ });

https://jsfiddle.net/muicss/4791kt3w/

I'm actually working on a new library that does something similar but it's still in development. Let me know if this works for you or if you're looking for different behavior.

@groupedsi
Copy link

Thanks a lot!
I'll keep an eye out for your new library and in the mean time I'll use the require function.

@amorey
Copy link
Member

amorey commented Sep 28, 2018

@groupedsi @naltatis Here's a new lightweight dependency manager I created called JohnnyDepp that implements the require method you requested:
https://github.com/muicss/johnnydepp

Let me know what you think.

@riklamme
Copy link

Johnny Depp is realy Nice. Why not loadjs2. This week i've write my own js class that uses loadjs to load dependency of a dependency. So i loaded jquery and lodash and a personal library, when a script require jquery and my personal lib. I gets a error from jquery not defined, so i maked a config with bundles and bundles has a prop files and when needed a dependency. Works as espected, but with depp on the horizon i refactor my class, tnxxx

@amorey
Copy link
Member

amorey commented Sep 28, 2018

Great! I'm happy to hear you like it. LoadJS and JohnnyDepp share the same underlying file loading code but their API's are different enough that I thought combining them into one library would be confusing.

JohnnyDepp is tested across all major browsers and should be production-ready so give it a try and let me know if it works for you.

@riklamme
Copy link

Was reading the code from Depp.js, Saw the same structurele, fetching is new functionality. So i change my code to Depp. Very Nice

@amorey
Copy link
Member

amorey commented Sep 29, 2018

Great! Let me know if you have any suggestions on how to improve the library.

@asos-tomp
Copy link

This lack of idempotency is a shame, I was really under the impression loadJs was functionally equivalent to $script.

The require() example is nice, but doesn't support the same script being requested via different bundle Ids, or handle different configuration options for numRetries, callbacks, etc. I'll try and knock up something that caters for that as an example.

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

6 participants