Use bem-xjst in Node.js without bundling via browserify (#407 fixed) #410

miripiruni · 2017-01-31T12:05:37Z

Fixes #407

Changes proposed in this pull request

use required bem-xjst instead of bundled
All public API not changed

API Demo (compile(), apply(), generate())

$ cat test.js
var bemhtml = require('./').bemhtml;
var libs = {
  jscs: require('jscs'),
  chai: require('chai')
};

var templates = function templates() {
  block('a').content()(function() {

    console.log(this.require('chai').version);
    console.log(this.chai.version);

    return applyNext();
  });
};

var htmlTemplates = bemhtml.compile(templates);

htmlTemplates.BEMContext.prototype.require = function require(libName) {
  return libs[libName];
};

htmlTemplates.BEMContext.prototype.chai = require('chai');

var bemjson = { block: 'a', content: 'First' };

console.log(htmlTemplates.apply(bemjson));

htmlTemplates.compile(function() {
  block('a').mix()('mixed');
});
console.log(htmlTemplates.apply(bemjson));

require('fs').writeFileSync('bemhtml-runtime.js', bemhtml.generate(function() {
  block('a').tag()('span');
}, { runtimeLint: true }));

$ node test
3.5.0
3.5.0
<div class="a">First</div>
3.5.0
3.5.0
<div class="a mixed">First</div>

$ node
> var b = require('./bemhtml-runtime')
undefined
> var t = b.compile()
undefined
> t.apply({block:'a'})
'<span class="a"></span>'
> t.apply({block:'a__b'})

BEM-XJST WARNING: wrong block name.
Block name can not contain modifier delimeter nor elem delimeter.
block: a__b
ctx: {"block":"a__b"}

>
(To exit, press ^C again or type .exit)
>

qfox · 2017-01-31T12:08:01Z

@miripiruni What you think about trying to bundle with rollup + rollup-plugin-node-resolve + rollup-plugin-commonjs? Guess we should get a try.

qfox · 2017-01-31T12:09:12Z

index.js

 var _cache = {};

-function getEngine(engineName) {
-  if (_cache[engineName]) return _cache[engineName];
+var getEngine = function getEngine(engineName) {


Do we really need var getEngine?

qfox · 2017-01-31T12:09:47Z

index.js

-  get bemtree() { return getEngine('bemtree'); },
-  get bemhtml() { return getEngine('bemhtml'); }
+  get bemhtml() { return getEngine('bemhtml'); },
+  get bemtree() { return getEngine('bemtree'); }


why you swap them? ;-)

@zxqfox I just wrote it from scratch, I can swap back =)

coveralls · 2017-01-31T12:10:24Z

Coverage increased (+0.4%) to 97.99% when pulling 2984d44 on issue-407__no-bundling into 53f0da9 on master.

qfox · 2017-01-31T12:11:39Z

lib/bemxjst/index.js

@@ -519,7 +524,8 @@ BEMXJST.prototype.exportApply = function exportApply(exports) {

  // Add templates at run time
  exports.compile = function compile(templates) {
-    return self.compile(templates);
+    self.compile(templates);
+    return exports;


Not sure it's a good behaviour at all, some times better to have immutable instances with templates.

@tadatuta what reason to do so? Only inconcistency between compiles? #408

Nah, just lack a way to the same as original compile — make an instance with templates in runtime or get templates from bundled one.

miripiruni · 2017-01-31T12:17:42Z

@zxqfox I’ll try rollup, thanks.

qfox · 2017-01-31T12:18:49Z

Looks good at all, but I'm not sure about bundled version. Don't know how to quickly check it

miripiruni · 2017-01-31T12:20:35Z

@zxqfox see PR’s description. I provide logs from my bundle test. Template bundling, bem-xjst options and etc works.

coveralls · 2017-01-31T12:23:05Z

Coverage increased (+0.4%) to 97.99% when pulling fc0da45 on issue-407__no-bundling into 53f0da9 on master.

qfox

/ok

miripiruni · 2017-02-01T15:29:45Z

index.js

-  var sourceBundle = fs.readFileSync(pathToBundle, 'utf8');
-
-  runtime.source = sourceBundle;
+  return _cache[engineName] = new Compiler(engineName);


miripiruni · 2017-02-01T15:31:52Z

lib/bemxjst/index.js

+BEMXJST.prototype.getTemplate = function(code, options) {
+  var templates = {};
+  this.compile(code, options);
+  this.exportApply(templates);


return this.exportApply();

miripiruni · 2017-02-01T15:33:09Z

lib/bemxjst/index.js

-
-  // Strip the function
-  out = out.replace(/^function[^{]+{|}$/g, '');
+  var out = code ? utils.fnToString(code) : '';


var out = utils.fnToString(code)

miripiruni · 2017-02-01T15:35:47Z

lib/compiler.js

+  if (options.runtimeLint) {
+    code = fnToString(code);
+    code = code + ';' + require('fs')
+      .readFileSync('./runtime-lint/index.js', 'utf8');


use require

miripiruni · 2017-02-01T15:36:40Z

lib/compiler.js

+  var api = new engines[this.engineName](options);
+
+  if (options.runtimeLint) {
+    code = fnToString(code);


code = fnToString() + …

miripiruni · 2017-02-01T15:40:53Z

lib/compiler.js


  var source = [
    '/// -------------------------------------',
    '/// --------- BEM-XJST Runtime Start ----',
    '/// -------------------------------------',
    'var ' + exportName + ' = function(module, exports) {',
-    this.runtime.source + ';',
+    bundleSource + ';',


заинлайнить

miripiruni · 2017-02-01T15:42:19Z

lib/compiler.js

-
-  var locals = this.runtime.prototype.locals;
+  var fs = require('fs');
+  var locals = require('./bemxjst').prototype.locals;


заинлайнить

coveralls · 2017-02-01T20:24:17Z

Coverage increased (+0.5%) to 98.048% when pulling e043e49 on issue-407__no-bundling into 53f0da9 on master.

miripiruni added semver: patch type: enhancement labels Jan 31, 2017

miripiruni requested review from tadatuta and qfox January 31, 2017 12:05

miripiruni added the status: review label Jan 31, 2017

qfox reviewed Jan 31, 2017

View reviewed changes

miripiruni force-pushed the issue-407__no-bundling branch from 2984d44 to fc0da45 Compare January 31, 2017 12:17

qfox approved these changes Jan 31, 2017

View reviewed changes

miripiruni commented Feb 1, 2017

View reviewed changes

miripiruni mentioned this pull request Feb 1, 2017

compile method of template instance should return itself #409

Closed

miripiruni force-pushed the issue-407__no-bundling branch from fc0da45 to fdca177 Compare February 1, 2017 16:50

Use bem-xjst in Node.js without bundling via browserify (#407 fixed)

e043e49

miripiruni force-pushed the issue-407__no-bundling branch from fdca177 to e043e49 Compare February 1, 2017 20:20

miripiruni merged commit 687969f into master Feb 1, 2017

miripiruni removed the status: review label Feb 1, 2017

miripiruni deleted the issue-407__no-bundling branch January 29, 2018 10:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use bem-xjst in Node.js without bundling via browserify (#407 fixed) #410

Use bem-xjst in Node.js without bundling via browserify (#407 fixed) #410

miripiruni commented Jan 31, 2017

qfox commented Jan 31, 2017

qfox Jan 31, 2017

qfox Jan 31, 2017

miripiruni Jan 31, 2017

coveralls commented Jan 31, 2017 •

edited

Loading

qfox Jan 31, 2017

miripiruni Jan 31, 2017

qfox Jan 31, 2017

miripiruni commented Jan 31, 2017

qfox commented Jan 31, 2017

miripiruni commented Jan 31, 2017

coveralls commented Jan 31, 2017 •

edited

Loading

qfox left a comment

miripiruni Feb 1, 2017

miripiruni Feb 1, 2017

miripiruni Feb 1, 2017

miripiruni Feb 1, 2017

miripiruni Feb 1, 2017

miripiruni Feb 1, 2017

miripiruni Feb 1, 2017

coveralls commented Feb 1, 2017 •

edited

Loading

Use bem-xjst in Node.js without bundling via browserify (#407 fixed) #410

Use bem-xjst in Node.js without bundling via browserify (#407 fixed) #410

Conversation

miripiruni commented Jan 31, 2017

Changes proposed in this pull request

API Demo (compile(), apply(), generate())

qfox commented Jan 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miripiruni commented Jan 31, 2017

qfox commented Jan 31, 2017

miripiruni commented Jan 31, 2017

coveralls commented Jan 31, 2017 • edited Loading

qfox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Feb 1, 2017 • edited Loading

coveralls commented Jan 31, 2017 •

edited

Loading

coveralls commented Jan 31, 2017 •

edited

Loading

coveralls commented Feb 1, 2017 •

edited

Loading