Add babel transpilation support and phantomjs browser tests (now also testing nunjucks-slim) #1053

fdintino · 2018-01-17T17:10:46Z

Summary

An overhaul of the bundle script and test runner, adding support for babel transpilation and phantomjs browser tests (the latter are instrumented with babel-plugin-istanbul and contribute to the coverage report). I'd particularly be interested in soliciting feedback from @ArmorDarks and @noahlange (as well as @carljm if he would like to weigh in).

Proposed change:

Bundling / build process changes:

webpack is run through babel-loader and uses the babel "env" preset to ensure the code runs on node > 4 and modern(ish) browsers. phantomjs is a decent target for the latter since it lacks support for most es6 syntax and features (this should include an update to CONTRIBUTING.md to bring it into alignment with the FAQ, which says we support whatever versions are currently supported by the Node.js Foundation Release Working Group).
The source files are moved from /index.js and /src/*.js to the /nunjucks subdirectory of the repo. During the npm run build process the original files are generated by babel-cli. This is to ensure that it doesn't introduce regressions if some developer has a deep import into nunjucks (e.g. var installJinjaCompat = require('nunjucks/src/jinja-compat')). The original un-transpiled source files are in .npmignore and the transpiled files are in .gitignore.
Added a postinstall script that uses postinstall-build so that npm run build executes if the dist files are missing. This removes the need to check the dist/browser files into git.
Related to the above: /browser/ files are removed from the repo and are now in .gitignore.

Test changes:

istanbul (which is now deprecated) is replaced by its successor, nyc
There is now a mocha browser test-runner for nunjucks-slim in /tests/browser/slim.html. When run through npm test it first precompiles the templates in /tests/templates/.
In addition to the already-existing mocha tests, npm test now also runs the mocha browser tests with phantomjs (though the mocha browser tests existed before this PR, they were not run in CI). Before and after executing phantomjs it starts and stops a simple static http server; the code for all of this can be found in /scripts/
(Very minor) mocha and expect.js are no longer vendored into /tests/browser, but are included from the repository's node_modules directory

Closes #1019 (had to be fixed for the slim tests to pass)

Checklist

Proposed change helps towards purpose of this project.
Documentation: not yet, will add an update to CONTRIBUTING.md
Tests are added / updated to cover proposed change.
Changelog—will add after feedback

fdintino · 2018-01-17T17:11:29Z

Another thing to note: I set eslint as the target branch so that the whitespace and linting changes can be reviewed independently of the more substantive changes in this PR.

ArmorDarks · 2018-01-17T18:03:33Z

Wow, that's a lot of work!

The source files are moved from /index.js and /src/*.js to the /nunjucks subdirectory of the repo. During the npm run build process the original files are generated by babel-cli. This is to ensure that it doesn't introduce regressions if some developer has a deep import into nunjucks (e.g. var installJinjaCompat = require('nunjucks/src/jinja-compat')). The original un-transpiled source files are in .npmignore and the transpiled files are in .gitignore.

Hm, that solution definitely imposing some issues.

It isn't okay to build into src dir. ES6 version should be kept there, It breaks expectations :)
Source files (ES6 ones) shouldn't be ignored.

The thing is, not all people would want to use transpiled version, mostly because CommonJS require and ES6 modules imports are different things. By importing transpiled CommonJS library you can't, for instance, use benefits of default export. Not to mention performance differences when you can use ES6 version directly

Thus, usually in src kept original ES6 version, which can be requested directly, and into another directory, like build or anything else served built version, for older Node versions and browsers.

This also explains why there is no point in adding ES6 version to .npmignore.

Now, there is an issue with supporting legacy users. I don't see any other solution except temporarily do movements in proposed way, but to plan within next major version bump to rename nunjucks into src and src into build or whatever you like. Or do major version bump now.

Added a postinstall script that uses postinstall-build so that npm run build executes if the dist files are missing. This removes the need to check the dist/browser files into git.

It would be better to use prepublishOnly script, which will be invoked during npm publish automatically. It should invoke build process.

Since built files shouldn't be ignored in .npmignore anymore, they will be included into NPM package. There is no need to build the library on the end-user side.

Note, that to build the library on the end-user side we need to force install development dependencies, which are completely useless for the end-user. So, that's not a good thing to do.

There is now a mocha browser test-runner for nunjucks-slim in /tests/browser/slim.html. When run through npm test it first precompiles the templates in /tests/templates/.

Wow, there is something really strange happens there. I have a strong feeling that it's possible to make it simpler.

Though, I can't tell straight away. Maybe it's just a Mocha ways to do this. I'm usually using Jest, which is basically Jasmine on steroids + jsdom. Due to jsdom it is much easier to tests such things. However, I'm not sure that it will allow testing memory leaks, because the environment is different.

fdintino · 2018-01-17T18:28:37Z

I'm heading to lunch so I'll reply in more detail to your comments (thanks for that, by the way!), but three quick notes:

If there isn't a concern about uses with the old import path (a github code search shows only a few) then I'm fine keeping the files in src and transpiling them into lib; however:
I intend to use es6 features such as object destructuring, classes, async functions, etc., but I have no intention of using es6 imports. I actually did that initially (the promise of es6 imports is that tree shaking can be more efficient because dependencies can be discovered via static analysis) but it did not seem to make a difference in the file size and to be frank I'm not a huge fan of the whole __esModule and exports.default stuff you get when going between commonjs and esm, so in the end I undid that. I don't really feel that the esm version:
```
import nodes from './nodes';
import {SafeString, Frame} from './runtime';
```
is any better than the commonjs version (with object destructuring):
```
const nodes = require('./nodes');
const {SafeString, Frame} = require('./runtime');
```
The reason that I think it is worthwhile to package babel transpiled source files in the npm package / as the main entrypoint in package.json is so that we can freely use newer syntax such as async / await (not available in node 6) or destructuring assignment / declaration (not available in node 4). I'm open to having different version support for source-installation vs. npm package installation. If you can point to a project that updates package.json "engines" and "main" as part of the prepublish step as an example I would be much obliged.

ArmorDarks

Also, there is another thing, related to bundling. Actually, we have to bundle more states of the lib than we do now.

Fortunately, I've found lightweight and quite self-explaining article regarding this: https://medium.com/@tarkus/how-to-build-and-publish-es6-modules-today-with-babel-and-rollup-4426d9c7ca71

The only thing which should be corrected, that today it is (almost) a standard to use module instead of jsnext:main field (some info)

Kinda living resemblance is https://github.com/reactjs/redux repository.

ArmorDarks · 2018-01-17T18:06:04Z

.babelrc

+  ],
+  "presets": [["env", {
+    "targets": {
+      "browsers": ["last 2 versions", "safari >= 7", "ie 9"],


It is better to replace this with single package.json. property:

"browserslist": ["last 2 versions", "safari >= 7", "ie 9"],

See https://github.com/ai/browserslist

In fact, it is a recommended by developers method.

ArmorDarks · 2018-01-17T18:07:34Z

.travis.yml

 node_js:
-  - "7"
+  - "8"


It is unclear why Node 7 no longer tested, as well as 5

node 5 and 7 are no longer maintained by the Node.js Foundation (even-numbered versions are designated for long-term support, but odd-numbered versions are only supported 2 months after a new LTS release comes out. The full details are on the nodejs blog post about the 8.0.0 release.

9 should probably be added though.

Ah, right. Fair enough.

ArmorDarks · 2018-01-17T18:08:30Z

.travis.yml

  - "4"
+install:
+  - npm install
+  - npm install codecov


why not install codecov as part of development dependencies?

ArmorDarks · 2018-01-17T18:12:04Z

package.json

+    "babel-core": "^6.26.0",
+    "babel-loader": "^7.1.2",
+    "babel-plugin-istanbul": "^4.1.5",
+    "babel-plugin-module-resolver": "3.0.0-beta.5",


Using custom resolver is close to cardinal sin. It will make code unusable for others, if they, for instance, will decide to require src directly. Unless it's used for something else.

Definitely, the aliasing functionality of this plugin is a terrible idea, but in my case I'm using it with webpack to remap certain packages for web (specifically: src/loaders.js becomes src/web-loaders.js and src/precompiled-loaders.js for full and slim builds, respectively), and also to map modules unneeded by the slim build to an empty module (e.g. src/precompile.js)

This config is specified in scripts/bundle.js and only applies to the browser bundles.

Currently on master this happens in bin/bundle.js, except it uses webpack.NormalModuleReplacementPlugin instead. I found that using that with babel caused weirdness in the code coverage results, and anyway I like the flexibility of being able to do browser / slim mappings in a callable.

I'm using it with webpack to remap certain packages for web (specifically: src/loaders.js becomes src/web-loaders.js and src/precompiled-loaders.js for full and slim builds, respectively),

That's sound like an issue with architecture, and resolver only makes things much less obvious.

Instead, a proper loader should be passed for proper cases at first place. This means that instead of remapping things, few versions of nunjucks/index.js should exist, which will import and use proper loaders and use or not use precompiler.

Shared functionality can be extracted into a standalone file.

Webpack should build each index.js version respectively.

There will be no need to remap things and it will become much easier to understand that Nunjucks supplies few different entry points. It will be even easier to tell how those entry points functionality differs just by looking onto imports.

That's a great idea. For whatever reason using separate entrypoints had not occurred to me, probably because I was thinking about how it used to be done, but it's a much more elegant solution (and would make jsdom-style testing easier too)

ArmorDarks · 2018-01-17T18:13:07Z

package.json

@@ -6,19 +6,50 @@
  "dependencies": {
    "a-sync-waterfall": "^1.0.0",
    "asap": "^2.0.3",
+    "postinstall-build": "^5.0.1",


We can avoid this dependency for end-user by using prepublishOnly script, as described in my issue comment

I don't think that's true if someone does a source install, e.g. npm install mozilla/nunjucks. The purpose of postinstall-build is to make npm install work with git installs without also having to move your build dependencies from devDependencies to dependencies

That is not da way.

If someone crazy enough to make NPM install directly from Github, he should realize consequences and should invoke building manually. That's is da way.

Adding a ton of dependencies just for building to already bloated node_modules of any user is a cardinal sin. Besides, a module can be consumed not only by NPM, but also by JSPM, where postbuilding is simply unavailable (probably).

I must not be explaining things properly. This 40K dependency (a 9K download from npm) makes it so that the build dependencies are not needed to npm install the package. Failure to do something like this would break npm install mozilla/nunjucks (or, more likely npm install userwithfork/nunjucks) which is a pretty common use-case. Otherwise I would need to commit the dist files to source control (bad) or move the build deps from devDependencies to dependencies (worse).

Ah, now I finally got what you're trying to do. I've read postinstall-build description and got an impression that you want completely move prepublishing build to the end-user. My bad.

Well, I see the point. Not sure that it worth it, but anyway, why not.

ArmorDarks · 2018-01-17T18:14:55Z

package.json

+    "uglifyjs-webpack-plugin": "^1.1.6",
+    "webpack": "^3.10.0"
+  },
+  "buildDependencies": {


This is something new. What's it used for?

This is a feature of postinstall-build. It allows you to specify a subset of your devDependencies necessary to run the postinstall hook (when doing a git npm install, for instance).

Oh my, I already imagine like npm install in Kotsu adding few other nice minutes of long waiting just while Nunjucks install all those dependencies to build itself...

This only happens if the installation is from git (the full command is postinstall-build src; the argument src is what changes the behavior: if the src folder is absent (which, based on your suggestions I will rename dist or lib) then it installs the build dependencies and calls npm run build, otherwise it does nothing). Does that address your concerns about build duration?

As I've already replied in #1053 (comment), I've got it totally wrong, so my objection above no longer valid :)

ArmorDarks · 2018-01-17T18:16:03Z

package.json

+    "build": "npm run babel && npm run bundle",
+    "codecov": "codecov",
+    "mocha": "mocha -R spec tests",
+    "instrument": "cross-env NODE_ENV=test scripts/bundle.js",


Would it be more obvious to call script bundle:test?

Or maybe test:bundle.

Same is for codecov and mocha. Probably would be better to group them under test:mocha and test:coverage

ArmorDarks · 2018-01-17T18:18:56Z

package.json

-    "lint": "eslint .",
-    "test": "eslint . && istanbul cover ./node_modules/mocha/bin/_mocha -- -R dot tests",
-    "browserfiles": "./bin/bundle"
+    "babel": "babel nunjucks --out-dir .",


I'd recommend to avoid binding to library names and instead call it transpile

ArmorDarks · 2018-01-17T19:08:32Z

If there isn't a concern about uses with the old import path (a github code search shows only a few) then I'm fine keeping the files in src and transpiling them into lib; however:

Well, we don't have all cases, because there could be private repositories. And even if there are none, it still feels like an ignorance of proper versioning. We're introducing a breaking change, after all.

Btw, I guess it is a more convention to build into dist or build. As I skim through repositories, I usually see that CommonJS version built into dist and ES6 version into es6

I intend to use es6 features such as object destructuring, classes, async functions, etc., but I have no intention of using es6 imports. I actually did that initially (the promise of es6 imports is that tree shaking can be more efficient because dependencies can be discovered via static analysis) but it did not seem to make a difference in the file size and to be frank I'm not a huge fan of the whole __esModule and exports.default stuff you get when going between commonjs and esm, so in the end I undid that. I don't really feel that the esm version:

We have no choice here. Sooner or later we will need to transition to ES6 modules since it is a standard. I'd say, it's better to do sooner just because it will save us time in the future.

Yes, I know that tree shaking isn't beneficial for Nunjucks right now. But it is right now. It might change in future.

Nevertheless, even if Nunjucks will never benefit from tree shaking, we're still obligated to use standards, which ES6 modules are.

The reason that I think it is worthwhile to package babel transpiled source files in the npm package / as the main entrypoint in package.json is so that we can freely use newer syntax such as async / await (not available in node 6) or destructuring assignment / declaration (not available in node 4). I'm open to having different version support for source-installation vs. npm package installation.

That's alright. I didn't propose to package non-transpiled version as a main entry point. See my comment #1053 (review)

If you can point to a project that updates package.json "engines" and "main" as part of the prepublish step as an example I would be much obliged.

Well, you can't :) Only one package.json can be published to NPM, so that won't work. And there is no need to do such thing. Unfortunately, there is no common agreement on how version and module package.json properties should co-exist. I need to dig deeper into understanding how Webpack and other supporting module property tools doing resolvement.

However, I don't see anything criminal it bumping Node version higher. In fact, we're obligated to bump to newer and safer versions.

fdintino · 2018-01-17T20:40:22Z

I take your point about needing to migrate to ES6 modules eventually, but for the moment I think "later" is more prudent than "sooner." Browsers are beginning to support it (though Firefox notably has the feature hidden behind the dom.moduleScripts.enabled setting in about:config), but node support for ES modules is still experimental, even with the latest release, 9.4.0. It only works if node is passed the --experimental-modules flag. In part that's because the feature proposal is still in DRAFT status. At least for now my compatibility preference would be to have the non-transpiled code able to run on the latest node LTS release (8.x right now) without babel, and have babel transpilation for compatibility with all releases supported as LTS by the Node.js Foundation. Until May that includes node v4, but after that point I'm fine dropping it—though I don't think it makes a practical difference since we'll still need babel to use ES2017 features for node 6.

I didn't decide against ES modules lightly, I actually went pretty far down the rabbit hole, even replacing webpack and uglify with jspm + systemjs / rollup and had the whole thing working. But in the end it proved to be a lot of trouble for not much gain—at least right now. On the plus side these changes to the build process make a transition to ES modules trivial.

One thing I'm not fully decided on is whether to transpile the files individually or whether to use webpack with target: 'node' to create a single file (perhaps /dist/nunjucks.js alongside /browser/nunjucks*), and whether to include sourcemaps in the package for the node dist file[s].

ArmorDarks · 2018-01-17T21:00:26Z

One thing I'm not fully decided on is whether to transpile the files individually or whether to use webpack with target: 'node' to create a single file (perhaps /dist/nunjucks.js alongside /browser/nunjucks*), and whether to include sourcemaps in the package for the node dist file[s].

Since there is no tree shaking, it mostly doesn't make difference. However, it will prevent users from requiring individual parts. If I'm not mistaken, let's say, I need only the parser — requiring parser will also pull whole nunjucks.js, right? Otherwise, I could simply require parser from an individual file, and it will pull only needed pieces of Nunjucks. Well, tree shaking, after all, makes difference here...

For instance, lodash provides both versions — and packed into single file lodash.js (main entry point) and all individual files.

Redux provides even more versions:

dist contains single-packed module redux.js and redux.min.js
es contains built individual files in ES6 format (module entry point)
lib contains individual files transpiled to work with CommonJS (main entry point)
src contains source. While included into package, it is mostly useless for distribution because it relies on development dependencies

Source maps shouldn't hurt anyone, I guess.

ArmorDarks · 2018-01-17T21:09:31Z

I take your point about needing to migrate to ES6 modules eventually, but for the moment I think "later" is more prudent than "sooner." Browsers are beginning to support it (though Firefox notably has the feature hidden behind the dom.moduleScripts.enabled setting in about:config), but node support for ES modules is still experimental, even with the latest release, 9.4.0. It only works if node is passed the --experimental-modules flag. In part that's because the feature proposal is still in DRAFT status. At least for now my compatibility preference would be to have the non-transpiled code able to run on the latest node LTS release (8.x right now) without babel, and have babel transpilation for compatibility with all releases supported as LTS by the Node.js Foundation. Until May that includes node v4, but after that point I'm fine dropping it—though I don't think it makes a practical difference since we'll still need babel to use ES2017 features for node 6.

That's why as the main entry point still transpiled version distributed. In other cases (like, say, with Webpack or JSPM) users almost always transpiling code anyway, so Node version doesn't mean anything for them.

I'm not insisting. It simply doesn't make much difference to argue about it. I'm just saying that it is unavoidable anyway.

I didn't decide against ES modules lightly, I actually went pretty far down the rabbit hole, even replacing webpack and uglify with jspm + systemjs / rollup and had the whole thing working. But in the end it proved to be a lot of trouble for not much gain—at least right now. On the plus side these changes to the build process make a transition to ES modules trivial.

Hm, didn't encounter any issues with them. They are mostly just... well... imports and exports. No gains, no loses. Just the way it should be done.

Issues are not within ES modules, but CommonJS. They are. But usually, they don't go as far as explaining in docs that for default export should be used .default property.

ArmorDarks · 2018-01-17T23:06:36Z

.npmignore

 docs
 tests
 bench
+nunjucks
+scripts


Worth noting, that instead of maintaining .npmignore it is easier to specify files property in package.json

ArmorDarks · 2018-01-25T16:41:08Z

.babelrc

+      "exclude": [
+        "es6.map", "es6.set", "es6.function.name", "es6.symbol",
+        "es6.regexp.split", "es6.regexp.replace", "es6.regexp.match",
+        "es6.string.repeat", "es6.object.set-prototype-of", "es6.regexp.flags"


I wonder what happens here. It seems like .babelrc config becomes more complicated than it should...

Babel is a bit over-agressive in including builtins, which balloons the size of the bundles. There are trade-offs in life: sometimes you get a more complicated babelrc to get a smaller js bundle.

I'd just say that excluding features this way is slightly dangerous. Using presets is a more accurate approach. But I guess since it is tested on older Node instances, it shouldn't be an issue...

ArmorDarks · 2018-01-25T16:41:38Z

.babelrc

+      "loose": true,
+      "useBuiltIns": "usage",
+      "targets": {
+        "browsers": ["last 2 versions", "safari >= 7", "ie 9"],


As already said, it is better to specify this in package.json browser list property.

Sure, there are several of your suggestions I haven't implemented yet. I'm just trying to make progress on this project. I'm not a huge fan of having a giant pull request with so many changes, so I'd like to get this merged into master sooner rather than later. At that point I can tackle the smaller things in more manageable PRs.

ArmorDarks · 2018-01-25T16:43:12Z

.eslintrc.js

-    'semi-spacing': ['error', {'before': false, 'after': true}],
-    'keyword-spacing': ['error', {'before': true, 'after': true}],
+  "rules": {
+    // The one assertion of personal preference: no spaces before parentheses


Oh no, not this again. Accepting common standards is hard, but it requires some courage and patience. It worth it.

I have comments explaining why I have disabled each of the rules. I did not do it without a good reason. And this top rule is the only one that I actually changed.

You might find this issue instructive. The people that maintain the airbnb style guide recognize that some rules are more controversial than others and that there are certain cases where disabling specific rules makes more sense than consistency for its own sake.

I did not do it without a good reason. And this top rule is the only one that I actually changed.

I can find a good reason for each rule why not to disable it. It is still a matter of personal preference, which plagues JS community. That's why striker tools, like Standard or Prettier, probably is a better choice. I'm just slightly disappointed that Nunjucks will continue that charade.

Anyway, arguing about codestyle is the same as arguing about tabs vs spaces. I've declared my concerns and they have been heard, I don't have anything meaningful to add. The choice is yours anyway to make.

ArmorDarks · 2018-01-25T16:45:11Z

.eslintrc.js

-      'sourceType': 'module',
-      'ecmaVersion': 2017
+  'extends': [
+    'airbnb-base/legacy',


As @noahlange reasonably said here #1052 (comment) taking Airbnb confit isn't a good idea if you want to use ES6+ features in Nunjucks. And since you do, taking an Airbnb config will result in a battle with it. Doesn't sound right.

airbnb-base/legacy is basically the airbnb rules without all the es6 stuff. It seemed like a good fit, it was the closest fit to my own personal preferences.

my own personal preferences

That's the thing which always troubles JS developers. Our own preferences. And so we desecrate the unification. Nevertheless, the project is your, so are the rules.

ArmorDarks · 2018-01-25T16:54:17Z

package.json

@@ -54,19 +58,23 @@
  "optionalDependencies": {
    "chokidar": "^1.6.0"
  },
+  "_moduleAliases": {
+    "babel-register": "@babel/register"


I wonder is it used anywhere? I see all instances pointing to the @babel/register...

I'll be able to remove it when istanbuljs fully supports babel 7.

ArmorDarks · 2018-01-25T19:01:41Z

You're doing a great job, which I and others were too lazy to do for years!

Sorry if I'm sounding too harsh at some points. Do not confuse it with lack of appreciation of your work.

This requires the Symbol.iterator well known symbol and Array.from to coerce them into vanilla arrays.

Add support for ES iterators

Using babel comes at a cost. In order to compensate, several tweaks are made in this commit to reduce the overall bundle size. These include: - Mangling all property names that begin with an underscore - Using simpler functions for Object.keys() and other ES6 features where we know that certain assumptions hold (that, for instance, we don't have to deal with symbols) - Using Obj.extend rather than class syntax when creating the Node classes In order to detect regressions caused by this more aggressive minification, the browser tests now use the minified bundle. The removal of niceties like Object.keys() is temporary until nunjucks v4, which will be distributed with a separate legacy build. The net effect is to decrease nunjucks-slim.min.js by 13K and nunjucks.min.js by 22K.

Having a single test for compiling operators that tests everything from addition and subtraction to python-style ternary operators support isn't very useful when something breaks.

fdintino · 2018-01-28T17:43:51Z

I'm going to open a separate ticket with this information, but thought it might make sense to share here first the plans for this PR and a proposed roadmap for nunjucks:

In this PR I've opted to be extremely conservative. I suspect that's mostly the cause of the decisions that @ArmorDarks has been (rightly) critical of. There are trade-offs I have had to make to ensure that I can release this as nunjucks 3.1.0 and be fairly certain that it won't break anyone's code. The idea is that I can immediately start on nunjucks 4 and have the 3.x branch be sufficiently similar that it's not too burdensome to backport (non-breaking) bug fixes.

In nunjucks 4 (the alpha release of which will follow quickly on the heels of merging this in) a few things will change:

I'll switch to a dist / es / lib / legacy / src directory layout, as suggested. The legacy folder will target node 4.x and old browsers. This will allow me to more freely use es6 features without being overly concerned in ballooning the size of the bundle with polyfills for anyone that doesn't need to support old versions of Safari or IE. (edit: I'm also open to the idea of requiring users who wish to target these browsers to integrate nunjucks into their own javascript build/babel pipeline and making the user responsible for having the polyfills in place).
I'll fix the scoping issues that might break code which relies on the non-standard behavior of nunjucks in order to bring it into alignment with the behavior of jinja2 and the other language ports. The code is littered with a bunch of strange assumptions about jinja2 scope that are simply incorrect, in particular that jinja2 scoping is somehow patterned off of python scoping. See these unit tests for example.

All of this will also set the groundwork for more ambitious goals for nunjucks. I would very much like to make nunjucks able to support plugins. The installJinjaCompat() function doesn't really cut it, and this will empower users of nunjucks to write their own Twig compatibility plugins rather than encouraging them to simply fork nunjucks and make custom changes. I'll be soliciting suggestions for how to architect this; the closest example I can think of that succeeds at this (in a different way) is babel. This might mean adding an AST transformation step between the parser and the compiler (so that nunjucks AST is converted to javascript AST, or something along those lines), before being converted to a rendered string.

I would also like to make nunjucks "slim" truly slim. Perhaps allowing the option of inlining the runtime functions and make precompiled templates able to run without needing to include the nunjucks library. This is just an idea, but if there is interest in it then I will build it.

ArmorDarks · 2018-01-28T18:46:19Z

Sounds awesome 👍

Fix pull request id in CHANGELOG.md

fdintino added 5 commits January 17, 2018 10:54

Add babel transpilation, phantomjs browser tests (with coverage)

fd310f7

Fix {% include ignore missing %} when used with nunjucks-slim

e0f1c33

Support jinja-compat with nunjucks-slim; fixes #1019

53597a2

Support running tests for slim mode

28b9cc3

Add codecov to CI scripts

d94db12

fdintino changed the title ~~Develop~~ Add babel transpilation support and phantomjs browser tests (now also testing nunjucks-slim) Jan 17, 2018

fdintino mentioned this pull request Jan 17, 2018

Switch from jshint to eslint, enforce 2-space indent rules, fix linting errors #1052

Merged

ArmorDarks reviewed Jan 17, 2018

View reviewed changes

fdintino added 7 commits January 25, 2018 11:24

Lint all files using a variant of airbnb-config

b4f0d57

Disable mocha and browser env on global eslintrc

19867d9

Fix express tests

c8d8f46

move express sample folder

013ea49

Fix bug that turned AsyncAll loops into AsyncEach loops

344f79f

Fix bug when looping over arrays of more than 3 dimensions

bdb5246

Split loop test into separate tests

e6cc590

fdintino force-pushed the develop branch from a0be771 to e6cc590 Compare January 25, 2018 16:24

ArmorDarks reviewed Jan 25, 2018

View reviewed changes

fdintino mentioned this pull request Jan 26, 2018

Add support for ES iterators #1058

Merged

4 tasks

maxnordlund and others added 4 commits January 26, 2018 21:12

Add support for ES iterators

7086e2d

This requires the Symbol.iterator well known symbol and Array.from to coerce them into vanilla arrays.

Merge pull request #1058 from maxnordlund/add-support-for-iterator

881e82d

Add support for ES iterators

tests: make compiler-operator and int/float filter tests more atomic

e16acda

Having a single test for compiling operators that tests everything from addition and subtraction to python-style ternary operators support isn't very useful when something breaks.

This was referenced Jan 28, 2018

v2.4.2 browser bundle does not properly export precompileString (and others) #785

Closed

for..in loop on a generator returns an empty string #861

Closed

ricordisamoa and others added 4 commits February 3, 2018 00:29

Fix pull request id in CHANGELOG.md

5138efd

Merge pull request #1061 from ricordisamoa/changelog

31d0ca4

Fix pull request id in CHANGELOG.md

Update CHANGELOG and CONTRIBUTING

84dfa2d

Bump version to 3.1.0

79b8ef3

fdintino merged commit 5ca49e8 into eslint Feb 20, 2018

fdintino deleted the develop branch February 21, 2018 16:13

Add babel transpilation support and phantomjs browser tests (now also testing nunjucks-slim) #1053

Add babel transpilation support and phantomjs browser tests (now also testing nunjucks-slim) #1053

Conversation

fdintino commented Jan 17, 2018

Summary

Checklist

fdintino commented Jan 17, 2018

ArmorDarks commented Jan 17, 2018

fdintino commented Jan 17, 2018 • edited

ArmorDarks left a comment • edited

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

ArmorDarks Jan 17, 2018 • edited

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

ArmorDarks Jan 17, 2018 • edited

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

ArmorDarks Jan 17, 2018 • edited

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

ArmorDarks commented Jan 17, 2018 • edited

fdintino commented Jan 17, 2018 • edited

ArmorDarks commented Jan 17, 2018

ArmorDarks commented Jan 17, 2018

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

fdintino Jan 25, 2018 • edited

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

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

ArmorDarks commented Jan 25, 2018 • edited

fdintino commented Jan 28, 2018 • edited

ArmorDarks commented Jan 28, 2018

fdintino commented Jan 17, 2018 •

edited

ArmorDarks left a comment •

edited

ArmorDarks Jan 17, 2018 •

edited

ArmorDarks Jan 17, 2018 •

edited

ArmorDarks Jan 17, 2018 •

edited

ArmorDarks commented Jan 17, 2018 •

edited

fdintino commented Jan 17, 2018 •

edited

fdintino Jan 25, 2018 •

edited

ArmorDarks commented Jan 25, 2018 •

edited

fdintino commented Jan 28, 2018 •

edited