BorisMoore / jsrender

A lightweight, powerful and highly extensible templating engine. In the browser or on Node.js, with or without jQuery.
http://www.jsviews.com
MIT License
2.68k stars 340 forks source link

ES6 Import #300

Closed ViktorHofer closed 8 years ago

ViktorHofer commented 8 years ago

Hey,

is there a way to include jsrender/jsviews with the es6 import statement? Currently I have to do it like this if I want to load jsviews with jquery not globally available:

require('./path/to/jsviews')($);

I would really love a way to do the es6 way: import './path/to/jsviews'; I also noticed that having the require statement multiple times in the code throws an exception (i think because of the function call after the require). I normally use browserify-shim when I wanna use a framework which requires jquery. Maybe that helps?

Thanks for your great work! 👍

BorisMoore commented 8 years ago

ES6 is not yet supported in browsers or on Node.js. I haven't studied how it will work, once it is broadly available. But right now we are concerned with Browserify and Node.js which both use require().

I don't know on ES6 import how one will be able to pass an instance of jquery to the imported jsrender.

Browserify-shim creates globals, right? I want people to be able to use jQuery/JsRender/JsViews without creating globals.

If you call require('jsrender') when there is a window.jQuery global it will return $. If you call it when there is no global, it returns a function to which you must provide the local $ as parameter.

So if you call browserify-shim and it adds a global jQuery then any subsequent calls to require('jsrender') will not return a function but $. Does that explain the exceptions being thrown (that you mention)? If not, can you provide a repro?

ViktorHofer commented 8 years ago

I will create a sample repo to reproduce the error. Just give me some days :+1:

Thanks for your time!

ViktorHofer commented 8 years ago

Okay I found the reason.

When jQuery is globally available (e.g. browserify-shim) but you require jsviews like that

const $ = require('jquery');
require('jsviews')($);

than jsviews will pick the global one. I think that jsviews should pick the argument if one is passed and if not then it should use the global one. The code changes would be these:

if (typeof exports === "object") { // CommonJS e.g. Browserify
        module.exports = function($) { // If no global jQuery, take jQuery passed as parameter: require("jsviews")(jQuery)
            $ = $ || global.jQuery;

            return factory(global, $);
        };
    }

But this is just something i noticed while searching for the issue. This isn't the root cause of the problem. The errors only happen when jQuery is globally available. I changed an existing library which needed to have jquery globally available to the modern approach and removed browserify-shim completely. Now the errors are gone...

Maybe that error description might help you... I don't now ...

Thanks so much for your effort! :+1:

BorisMoore commented 8 years ago

The problem with that approach is that now require('jsviews') (and require('jsrender') if the same pattern is used for that) will always return a function, so if window.jQuery is defined, and you write:

var jsrender = require('jsrender');
jsrender.templates(...); // This will fail

or

var $ = require('jsviews');
$("#foo")...; // This will fail

-- both will both fail - since jsrender and $ will have been set to the function.

People would have to write:

var jsrender = require('jsrender')();

or

var $ = require('jsviews')();

This is actually the most common scenario, so I think requiring those extra parens is going to confuse a lot of people...

BorisMoore commented 8 years ago

Closing