We need to ensure that new PR's do not introduce type issues, I see the path for this something like the following:

• Add npm run vscode-build to .travis.yml's script (this will get at least a pass/fail for PR's until better testing can get setup).
• Incorporate broccoli-tslinter to enable live reload style testing.

I'd like to begin standardizing the template error structure returned by all of the various Ember components such that we can better utilize the plethora of information that is known at the time the error is thrown.

One of the major end results of this is the ability to generate a very user-friendly error page to developers: ember-cli/broccoli-middleware#4 (comment)

The error structure is still up for discussion, but will be documented here along with the work required on the various other parts of the Ember ecosystem.

Some of the notable locations here in Glimmer are:

• https://github.com/tildeio/glimmer/blob/46656f09030bcfcb22c825f3585cb667eb5c0fe2/packages/glimmer-syntax/lib/utils.ts#L29
• https://github.com/tildeio/glimmer/blob/46656f09030bcfcb22c825f3585cb667eb5c0fe2/packages/glimmer-syntax/lib/utils.ts#L37
• https://github.com/tildeio/glimmer/blob/c16fe9e2fe16860583d74de701e5245f85f15709/packages/glimmer-syntax/lib/parser/handlebars-node-visitors.ts#L22
• https://github.com/tildeio/glimmer/search?utf8=%E2%9C%93&q=path%3Apackages%2Fglimmer-syntax+throw&type=Code

/cc @rwjblue @stefanpenner

It has been observed that the source map lines tend to "drift" over time (flushing the caches and/or restarting the server seems to help)

The blockMeta argument should be added to getComponentDefinition here, and threaded through the various opcodes that need it.

• if
• unless
• with
• each
  • ...PutIterable?
• component
• partial
• in-element
• dot path deopt?

0e83fa3

The check should happen inside the TestOpcode, and it should delegate to the Environment for the specific definition of "truthy-ness"

There should be an easy way to assert that a re-render does not re-create DOM nodes

Godhuda is taking this today

The import paths for the index.d.ts files are incorrect in the transpiled output.

The transpiled structure is:

% tree dist/es6/ -L 1
dist/es6/
├── htmlbars-compiler
├── htmlbars-compiler.js
├── htmlbars-object
├── htmlbars-object.js
├── htmlbars-reference
├── htmlbars-reference.js
├── htmlbars-runtime
├── htmlbars-runtime.js
├── htmlbars-syntax
├── htmlbars-syntax.js
├── htmlbars-test-helpers
├── htmlbars-test-helpers.js
├── htmlbars-util
├── htmlbars-util.js
├── htmlbars.js
├── simple-html-tokenizer
└── simple-html-tokenizer.js

The contents of htmlbars-compiler.js is:

import {
  compile,
  compileSpec,
  template,
} from "./lib/compiler";

export { default as TemplateCompiler } from './lib/template-compiler';
export { default as TemplateVisitor } from './lib/template-visitor';

export {
  compile,
  compileSpec,
  template
};

There is no ./lib folder in this context. The tests happen to work now because the loader being used had a bug.

I'm assuming the issue was introduced here: #198
https://travis-ci.org/tildeio/glimmer/builds/140541684

cc @rwjblue

This is also an issue in HTMLBars, but I wanted to record the issue here (as it is unlikely that we would make many changes in tildeio/htmlbars).

Both TextNode and CommentNode are built without loc, and loc is not populated during finishComment or finishData.

Some links:

• b.text without loc -- https://github.com/tildeio/glimmer/blob/master/packages/node_modules/glimmer-syntax/lib/parser/tokenizer-event-handlers.ts#L27
• b.comment without loc-- https://github.com/tildeio/glimmer/blob/master/packages/node_modules/glimmer-syntax/lib/parser/tokenizer-event-handlers.ts#L13
• buildComment doesn't accept a loc param -- https://github.com/tildeio/glimmer/blob/master/packages/node_modules/glimmer-syntax/lib/builders.ts#L46-L51

Looks like glimmer is already used on NPM: https://www.npmjs.com/package/glimmer

What should we use when publishing this?

If the name of the partial is knowable during template scanning (as is the case with {{partial "foo"}}) we should inject the statements of the partial directly into the block the contains the partial at scan time.

This will avoid creating several unnecessary opcodes that are used to track the name of the partial in the dynamic case {{partial partialName}} and make the performance of a static partial as if you had copy and pasted the template (plus a small, constant overhead of calling lookupPartial).

It appears at fdc0575 initial render has regressed some. This is pretty non-scientific (using uptime-boxes), the regression may be unavoidable but noticed the jump from 400s to 500s.

Initial render: 40db3bc
Mean: 468.7
SD: 9.33393
Variance: 87.12222

464
491
464
462
461
476
461
473
469
466

Initial render: 81aaa30
Mean: 536.4
SD: 17.30896
Variance: 299.6

566
522
533
515
529
550
515
557
536
541

#50 removed the identity test due to the fact that they were implemented as a side effect of eagerly compiling layouts.

Spec

• Inside of the <foo-bar /> component you should be able to use a <foo-bar /> custom element
• Compiling the <foo-bar /> layout should output OpenElementPrimitive for the top level element if it is an identity element

With the new Ember.js 2.10 release which ships Glimmer by default, there is one small downside.

https://github.com/kategengler/ember-cli-code-coverage
Nicely showed code coverage for templates pre Ember.js 2.10, with the new output, no useful coverage of templates is collected.

Are there any plans of instrumenting Glimmer-Templates for code coverage?

e.g. EmberIDs

#158 is the wrong fix. We introduced the problem around #149. The problem is in glimmer-demos/index.ts, we re-export stuff from the bench* files. This is problematic because we try to access the Benchmark global variable here as soon as you require that file. You need to load the benchmark.js on the page for that global to be available, which is why adding the script tag to everything fixes it.

However, since all the demos don't otherwise use Benchmark library, adding the script tag to everything is a bit silly. Instead, we should move all the benchmark stuff in to a new, top-level glimmer-benchmarks package and make the page available in https://localhost:4200/benchmarks.html or something. We already did the work to make it a single HTML page – we can include benchmark.js in that page but we won't have to do it in the other demos. Since demos have no reason to require the benchmark stuff, it won't have need to access the Benchmark global anymore.

When registering a component without a JS class, in theory we don't need to create a component instance for it (and call hooks, etc). Probably should measure the impact before going down this path.

When I bump ember's glimmer-engine version to any SHA after 0fdf041 I get the following error upon running npm start in the ember.js repo:

$ npm start

> [email protected] start /home/zmulgrew/workspace/zth-ember.js
> ember serve

DEPRECATION: Overriding init without calling this._super is deprecated. Please call `this._super.init && this._super.init.apply(this, arguments);` addon: `ember-cli-dependency-checker`
    at Function.Addon.lookup (/home/zmulgrew/workspace/zth-ember.js/node_modules/ember-cli/lib/models/addon.js:896:27)
Could not start watchman; falling back to NodeWatcher for file system events.
Visit http://ember-cli.com/user-guide/#watchman for more info.
Could not require 'ember-cli-build.js': Cannot find module 'broccoli-tslinter'
Error: Could not require 'ember-cli-build.js': Cannot find module 'broccoli-tslinter'
    at Function.Module._resolveFilename (module.js:325:15)
    at Function.Module._load (module.js:276:25)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/home/zmulgrew/workspace/zth-ember.js/node_modules/glimmer-engine/ember-cli-build.js:11:14)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Module.require (module.js:353:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/home/zmulgrew/workspace/zth-ember.js/ember-cli-build.js:45:21)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)

I believe this is because broccoli-tslinter is listed as a devDependency in glimmer, but the ember.js ember-cli-build.js requires glimmer-engine's ember-cli-build.js:

var glimmerEngine = require('glimmer-engine/ember-cli-build')();

Not sure what the best way to fix this is? I can think of two options:

1. Move broccoli-tslinter to dependencies in glimmer.
2. Add broccoli-tslinter- to ember.js's devDependencies so it will "be there" after npm install.

Ideas?

@chadhietala @GavinJoyce

Previously removed in #19

Re-rendering a dynamic partial always results in the DOM being invalidated, even when the partial is the same. Working on fixing this.

http://emberjs.com/api/classes/Ember.Component.html#property_positionalParams

76b4c42

We will rework the implementation tomorrow, and add tests

https://github.com/tildeio/glimmer/blob/master/packages/glimmer-runtime/lib/dom.ts#L89

If bar === “” on initial render (or later become empty), it seems like we won't be able to update it after. Should probably put an empty comment in there.

I would like to restructure the tests for browser-specific DOM-patches (see bdb5512).

Goals:

• Clearly document the browser-specific quirks and our hax
• Good coverage for these quirks – they are hard to notice (I don't test on IE9 locally) and easy to break
• Clearly flag the hax/tests so we know what to remove when we drop support for certain browsers

Specifically:

• A QUnit module (or test file? even a folder?) for each quirk/hax
• Move the existing innerHTMLFix test cases to the module
• One test case for each of the affected elements (<table>, <tbody>, ...), probably DRY-ed up with a helper
• Test update as well as initial render
• Verify if we need to care about the other elements mentioned in the comment (<style>?)

Now that caching for layouts is done by consuming env we should implement this caching in the TestEnvironment so our benches and demos don't artificially regress.

1. Checked out glimmer
2. Ran npm install
3. Started server with npm start
4. Opened demo http://localhost:7357/demos/visualizer.html

On demo page you will get an error:
loader.js: 81 Uncaught ReferenceError: Benchmark is not defined

I think it may be time to move away from objects as "opcodes" (hmmm sounds weird) and instead just use an array of strings. Because the "opcodes" are megamorphic the append on the LinkedList never JITs making it very slow. Using an Array and slice is likely better here.

/cc @krisselden

In the compiled output we currently send down a lot strings that get mapped into runtime syntax objects. While this works and is an extremely large improvement over Glimmer1's output we still have headroom here for further optimizations.

In the short term we should keep an enum in Glimmer that maps an opcode name to a string. In the long term we may want to investigate going further down the byte-array path.

IE bug report here: https://connect.microsoft.com/IE/feedbackdetail/view/944330/invalid-argument-error-when-changing-nodevalue-of-a-text-node-removed-by-setting-innerhtml-on-an-ancestor

Basically, if an element's innerHTML is replaced before glimmer tries to insert text (due to the element having handlebars markup inside of it), IE11 errors out.

The line that errors is here: https://github.com/tildeio/glimmer/blob/3437ee92284317f3bf32b661cb1089e2915f7213/packages/glimmer-runtime/lib/upsert.ts#L75

Wrapping that line in a try/catch seems to work and give the same functionality as in Chrome (the original innerHTML modification takes precedent, because glimmer is modifying an non-existent node).

EDIT: Wrapping it in a try/catch, and returning false in the catch does not work, but returning true does.

The best way to exercise that hook is probably to add a simple "closure component" setup in Glimmer. It doesn't need to do the complicated merging setup in Ember, but something like first (or last) invocation wins is probably sufficient. (We just need to test that you can indeed change the args to something other than what is supplied in the invocation.)

tests for #229

cc @chancancode / @chadhietala

Related to #50 and #51.

Spec

• You should be able to recursively invoke a component.

Apologises if this is the wrong place to ask this, but I watched the awesome talk by Godfrey & Yehuda on the upcoming changes to glimmer, and I couldn't help but notice you guys were representing your op code as a series of arrays and strings. I had a further look at the source and (I think) that is the case. Have you considered outputting the opcode in the form of a typed array?

Most of the performance boosts would come from the fact Typed arrays have a contingent memory layout, which could likely lower memory usage, provide an additional execution boosts, and could decrease the overall network footprint. The main drawback I can think of is the lack of IE 9/8, or Opera mini support.

Thoughts?

Currently, TestEnvironment#registerComponent eagerly compiles the component layout, which causes it to try to lookup other component definitions that might not have been loaded yet (which then causes the compiler to emit OpenPrimitiveElement).

(This is just a TestEnvironment implementation issue, so it shouldn't require any Glimmer changes)

Glimmer has handlebars ^3.0.1 support but some of the niceness of decorators and block partials are missing.

I changed the package version to ^4.0.0 so my fork would let me use Handlebars 4 but threw an error when I tried using

{{#*inline "myPartial"}}
  My Content
{{/inline}}

where the error was

parser.ts:52 Uncaught TypeError: this[node.type] is not a function

Is there a possible future for using ^4.0.0 (and 5.0.0 someday)?

I've mentioned this before in emberjs/ember.js#12200, seems like we may want to do this in the new object system. While I understand that it may hinder addons, it will be more of a foot gun when async engines are a thing and you lazy load some code that potentially modifies the prototype of all the classes already held in memory.

Don't want to get into scope creep, but I think this needs to be done regardless of new object system.

See relevant issue at ember-cli-fastboot: ember-fastboot/ember-cli-fastboot#258

Referencing Node.DOCUMENT_FRAGMENT_TYPE in dom-helper's isDocumentFragment method causes an issue when rendering an ember app using ember canary (2.9.0-alpha.4) and ember-cli-fastboot (1.0.0-beta.8) because the global Node is not available in the Node.js environment.

I'm not certain if this is an oversight in glimmer, or if something else should be happening to make it so that ember-cli-fastboot is using the exports from node-dom-helper.ts instead.

When #242 updated the tests/index.html to require the .tslint files, a number of modules were indicated as failing.

We need to uncomment this line and fix any linting errors that show up.

https://github.com/rwjblue/broccoli-ember-inline-template-compiler

1. produce a standalone template compiler JS file
2. provide that tot he inline-template-compiler
3. update tests to use that.

• Make tests runnable #10
• Ensure ES6 output for publishing has correct imports #6 and #7
• Rename all packages from htmlbars-* to glimmer-*
• Create package in Ember for ember-glimmer branch here
• Create feature flagged builds in Ember, swapping glimmer2 engine vs glimmer1 engine based on flag
• Ensure tests pass for Ember with and without the feature enabled

• Determine if #9 is correct, or lock down to different set of dependencies if not. -- @wycats / @chancancode
• Determine which tests are considered passing. -- @wycats and @chancancode
• Mark any other tests that do not pass as QUnit.skip -- @rwjblue