File weight

[esjewett]

Hi,

We're embedding YASQE and YASR into the Palladio project (they should appear in the next version thanks to @jiemakel ). At the moment they are going to be our heaviest dependency. I haven't looked into it deeply enough to understand exactly why this is. I've had experience with Browserify embedding all dependencies in the bundle, so I wonder if that might be happening.

So, really 2 questions: Would you be open to a pull request to provide a distribution without dependencies? And, if you are open to that, do you have any ideas about where I should start?

Thanks! Ethan

[LaurensRietveld]

Hi Esjewett,

Ofcourse, go ahread. I'd welcome a pull request. Indeed the issue is in browserify, which bundles all dependencies to the single distribution files.

As far as YASQE is concerned, the biggest culprit is the SPARQL grammar. (see lib/flint.js) This is the SPARQL grammar made by the people from DERI. Defined in prolog, and compiled to javascript. This grammer is extremely useful (practically all of the YASQE features depend on it), but extremely verbose and bloated as well...

The only alternatives as far as this library is concerned are to either

• optimize the current js grammar generation (see here for how the file is created now: https://github.com/TSO-Openup/FlintSparqlEditor/tree/master/ll1)
• or to change from this grammar, to the SPARQL.js library made by Ruben Verborgh (https://github.com/RubenVerborgh/SPARQL.js). This one is -a lot- smaller, and from a first glance looks like it could do the job I'm keen on using this library, but to be honest, I won't have time to do anything other than bug fixes before the end of this year...

gr Laurens

[esjewett]

Thanks for the pointers. I had noticed that grammar and was trying to figure out what it was. Thank you for the explanation. (I am not very knowledgeable about SPARQL.)

I'll put together a pull request for each library (YASQE and YASR) in the next week or so that will leave the existing distribution files the same but add a couple of others that don't include the dependencies. Should be minimally disruptive.

[esjewett]

Quick update: I did some initial work on this. The basic idea was to use browserify-shim to shim out dependencies from the bundle. Works great for jQuery. Unfortunately there is something about the CodeMirror add-ons that is making it impossible to get rid of CodeMirror this way. I've asked on StackOverflow, but so far to no avail. I'm sure there must be a way to do this...

I'll wait a bit longer for a response and keep mining my network to see if I can get some guidance, but thought I'd report back here in case you had any ideas.

[LaurensRietveld]

that seems like a tricky problem indeed. What about simply not using browserify to bundle the addons, but minify/concatenate (prepend) these addons to the generated browserify bundle? (and refer to these global addons using shim) Not sure if it'll work though, and it surely isnt the cleanest solution.

[esjewett]

That's a good idea. It would be a little weird because the add-ons would magically be "there" without being required in the actual YASQE javascript files, but it could solve the problem.

[esjewett]

It looks like this works. The add-ons will fall back to using the global 'CodeMirror'. I had to change the gulp process in order to be able to use merge-stream, but I think it's actually simpler now. To get rid of jQuery I'm going to first have to change yasgui-utils. I will eventually send a separate pull request for that, then jQuery should just fall out of YASQE as well.

[LaurensRietveld]

excellent, just saw your pull request, I'll make sure to merge it this week. About yasgui-utils: I'm actually surprised jquery is in there.. (I could just as easily have programmed the 'determineId.js' functionaly using the regular DOM tree)

[esjewett]

The issue is that yasgui-utils includes a require('jquery') IIRC, and because browserify-shim is apparently not able to shim dependencies of dependencies, this pulls in jQuery during the process of browserifying YASQE :-/

[LaurensRietveld]

I meant something more in the line of removing jquery from utils alltogether. Jquery is only used for 1 command:

return require("jquery")(element).closest('[id]').attr('id');

The following should work as well, without needing to include jquery:

if (element.closest) {
    return element.closest('[id]').attr('id');
} else {
    var id = null;
    var parent = element.parentNode;
    while (parent && id == null) {
        parent = parent.parentNode;
        if (parent && parent.getAttribute('id') && parentNode.getAttribute('id').length == 0) 
            id = parentNode.getAttribute('id');
    }
    return id;
}

Haven't tested it though ;). If you can wait one or two days (got a paper deadline atm) I can test and add it to the utils. (or if you want you can try it out on your own fork)

[esjewett]

That would be great! I may get to it over the weekend if you haven't been able to look at it at that point. Thank you!

[LaurensRietveld]

Quickly added the code above to the new utils version. Could you let me know if there are any issues?

[esjewett]

Getting there. Looks like it is still pulled in based on the require in imgs.js? I think the usage there is pretty basic and could be replaced by calls standard DOM APIs. Just a guess:

//$(parent).append(el);
parent.appendChild(el);

//$("<div></div>").css("display", "inline-block");
document.createElement("div").setAttribute("style", "display:inline-block;");

//svgContainer.width(config.width).height(config.height);
//return svgContainer.append(svg);
svgContainer.setAttribute("width", config.width);
svgContainer.setAttribute("height", config.height);
svgContainer.appendChild(svg);
return svgContainer;

Think that will work?

[esjewett]

Oh boy. Just realized that I think setting the browserify option bundleExternal=false basically solves this. I tried this previously but in the options passed to bundle() and it needed to be done on the options passed to browserify(file, opts). May be able to roll back some of this, but I need to verify it's doing what I think it is (just not bundling stuff from the node_modules directory).

[LaurensRietveld]

Ah, that would be even better indeed. Let me know how it goes ;) (I'll revert the utils change then for now)

On Thu, Oct 9, 2014 at 2:05 PM, Ethan Jewett notifications@github.com wrote:

Oh boy. Just realized that I think setting the browserify option bundleExternal=false basically solves this. I tried this previously but in the options passed to bundle() and it needed to be done on the options passed to browserify(file, opts). May be able to roll back some of this, but I need to verify it's doing what I think it is (just not bundling stuff from the node_modules directory).

— Reply to this email directly or view it on GitHub https://github.com/YASGUI/YASQE/issues/2#issuecomment-58499363.

[esjewett]

I reworked the pull request with bundleExternal=false, which helps but doesn't solve the problem as it leaves those 'require' statements for external modules in there. So if the downstream project isn't using browserify, those are errors. I used a combination of forcing requires (for the CodeMirror add-ons), shimming using browserify-shim (to get rid of the requires in main.js), and hardcoded gulp-replace statements to remove the 'require' statements in yasgui-utils that browserify-shim can't get at.

Tested that this works in Palladio with both the all the .js files in the /dist directory.

I'll provide a bower.json eventually as well, based on what we're using in Palladio. And we can probably tackle the YASR bundle using the same approach.

[LaurensRietveld]

too bad that we'd need the gulp-replace statement, which may be a bit fragile. (e.g. someone changes single quotes to double quotes, and replacement does not work anymore?) Why are these needed for yasgui-utils? Is it to get rid of the jquery?

And I'll update the codebase tonight probably. Made a dev branch, where I'll merge it to

[esjewett]

Yup, it's to remove the dependency on jquery and on store. Those dependencies can be indicated in the package.json for YASQE. Ideally yasgui-utils would have a browserify build with its own browserify-shim for jquery and store (maybe using browserify-shim's browser option instead of globals), then those dependencies could be moved to yasgui-utils package.json and we could remove the gulp-replace statements.

[LaurensRietveld]

oke, I'll see what I can do (I'll probably remove the jquery dependency altogether from utils). I'll let you know when I've merged everything (did some other changes to my gulp files in the meantime, so will have to do some conflict resolution ;))

[esjewett]

I'll be able to take a shot at yasgui-utils as well, but probably not this week. Nice thing is that if you remove or shim the dependencies in yasgui-util, the gulp-replace steps will just stop doing anything.

[LaurensRietveld]

I've update yasgui-utils: jquery is not used anymore. Tbh, I don't care that much about creating a shim for the storage library: its pretty small, and I don't think lots of people use this in similar projects as well (i.e., we'll only create overhead for people who need to fetch and include it separately)

I've also merged the pull request, and pushed it to a temp public branch: https://github.com/YASGUI/YASQE/tree/esjewett-dependency-work But, there are some issues w.r.t. codemirror: the require does not work when including the 'dep' js file. (try the index.html file in the branch) Not sure whether something went wrong with the merge, or whether this is a different issue. Could you have a look to double-check?

[esjewett]

I'll check out the dep.js file issue.

Regarding the storage library, that's definitely fine. But in that case it should be packaged with the yasgui-utils bundle using browserify, shouldn't it? Without doing that, it seems to me that the require("store") isn't going to work in any app using YASQE that isn't using a CommonJS-based framework (like Browserify).

[esjewett]

I believe I've addressed these issues in pull request #15 against the branch. This is really kind of nuts, but we do get a 50% reduction in minified file size! :-)

[LaurensRietveld]

Great, thanks! About the utils library: my intention behind this library is not the direct use in browserify by third parties, but specifically a place to put code which is shared between YASQE and YASR (to avoid duplicate codebase). Perhaps in the future if it becomes something more than a place to store shared code, I'll think about making it into a proper library (without e.g. the browserify requirement)

I've also merged your pull request. Everything was working fine, but I've refactored some stuff anyway: For maintainability reasons, I'd like to avoid updating 3 or 4 files whenever I add e.g. a codemirror addon to the codebase. Therefore, I was looking for a way to build the dependency-free version based on exclusion, instead of inclusion. And, I found that for minified versions, the merged files messed up the map file created by browserify (which is useful for debugging).

Anyway, the new version has a few dev dependencies left, and uses the 'exclude' browserify function for jquery and codemirror. It seems to be working well, but not sure whether I've missed something. I've also added another index (index-deps.html) file for testing, which uses a cdn for jquery and codemirror, and uses the dependency-less version (yes, I see now that the name of the new index file should have been different ;))

This looks like it is about done now right? Would you mind checking it out one more time to see whether you spot anything amiss?

[esjewett]

I might be missing it, but it seems like you might have forgotten to push your latest changes, as I don't see the use of browserify's exclude function.

I didn't think about the mapping. Probably your way of doing it with exclusion is better. Hopefully it will still work! I should be able to test fairly quickly after you push your updates.

[LaurensRietveld]

Oops, jup, forgot to push. its there now

[esjewett]

So, I just tried the latest version of the esjewett-dependency-work branch and after running gulp it looks like we're back up to 356KB for the yasqe.min.js. I believe this is because CodeMirror is still getting pulled in via the CodeMirror add-ons. You'll need to add the .exclude('../../lib/codemirror') to fix this.

index-deps.html is also broken because it can't find the jquery module. I think it expects it to be packaged with Browserify. You'll need to add the browserify-shim back in the package.json and add the transform(browserifyShim) thing back to the dependency-free build. But I don't think that's enough because you'll run into an error around not being able to find the module "../../lib/codemirror", which we can't shim :-(

[LaurensRietveld]

Yes, your right, and we're back to square one. I'll sleep on this one, and figure it out tomorrow ;)

[LaurensRietveld]

Ok, an update: there are browserify transform plugins which will do what we'd like, e.g. https://github.com/benbria/aliasify , or even https://www.npmjs.org/package/browserify-transform-tools itself. These allow you to change require statements. In our case, from ../../lib/codemirror to 'codemirror'. Only thing I haven't figured out yet is how to apply these to transitive includes. I got these tools to work on the includes in src/main.js (on which browserify is executed), but we'd need to change the require -in- several of these includes. To be continued

[LaurensRietveld]

I think we're there, though you might want to check again (I was wrong the previous time as well ;)). The problem we had was that browserify does not apply the 'transform' plugins recursively: it only executes these on the requires which are defined in the file(s) you try to browserify (see here for a lengthy discussion about this: https://github.com/substack/node-browserify/issues/501) We are -extremely- lucky, that last week the node-browserify package was updated to 6.1, where there is support for a global transform. We don't have to use any other transform libraries such as aliasify to fix this relative path inclusion of codemirror. Instead, we can use the browserify-shim module, and shim '../../lib/codemirror' to 'global:CodeMirror'. The shim is not registered in package.json to run as a transform, but explicitly added as transform with global:true option.

Would you mind checking again whether this is what we are looking for?

gr Laurens

[esjewett]

Getting there. Moving the settings around broke the UMD bundling, so you get an error about YASQE not being found. Should change it back to:

var baseBundle = browserify("./src/main.js")
        .transform({ global:true }, shim)
        .exclude('jquery')
        .exclude('codemirror')
        .exclude('../../lib/codemirror')
        .bundle({ standalone: "YASQE", debug: true })

And the equivalent change in the version for bundling with dependencies. Thanks for figuring out the global transform had just been released. That's exactly what we needed to deal with those add-ons!

[LaurensRietveld]

That's strange, here it still works. You might need to run npm update, to make sure you have the latest browserify version (this version should complain about the bundle options: in the new version it won't allow options as argument to the bundle command. Instead, these options should be passed on to the browserify constructor)

[esjewett]

Yup, you are completely right. I forgot to run 'nom update' and was still on the old version of browserify. I think we're good to go!

[LaurensRietveld]

Excellent! I'll do some minor changes to the README, and will probably create 4 different builds: complete, yasqe+jquery, yasqe+codemirror, yasqe (as most people will have jquery included, but not codemirror). I'll then merge this to the master branch. Thanks for helping out!

On Wed, Oct 15, 2014 at 7:33 PM, Ethan Jewett notifications@github.com wrote:

Yup, you are completely right. I forgot to run 'nom update' and was still on the old version of browserify. I think we're good to go!

— Reply to this email directly or view it on GitHub https://github.com/YASGUI/YASQE/issues/2#issuecomment-59244071.

[esjewett]

Thank you! What a headache. I wouldn't have been able to figure out how to do this without your help.

[LaurensRietveld]

Yes, this was a very (unexpected) tricky problem indeed..

On Wed, Oct 15, 2014 at 7:39 PM, Ethan Jewett notifications@github.com wrote:

Thank you! What a headache. I wouldn't have been able to figure out how to do this without your help.

— Reply to this email directly or view it on GitHub https://github.com/YASGUI/YASQE/issues/2#issuecomment-59244898.