Integrate Glimmer Macros For Binary VM #14781
Conversation
| compile(builder) { | ||
| builder.component.dynamic(this.definitionArgs, this.definition, this.args, this.symbolTable, this.shadow); | ||
| } | ||
| export function inlineComponentMacro(path, params, hash, builder) { |
There was a problem hiding this comment.
This likely can go away in favor of dynamicComponentMacro.
|
☔ The latest upstream changes (presumably #13549) made this pull request unmergeable. Please resolve the merge conflicts. |
chadhietala
changed the title
chadhietala
force-pushed
the
macros
branch
3 times, most recently
from
fb168b2
to
d305684
Compare
| annotation: 'strip sourceMappingURL' | ||
| }); | ||
| } | ||
| // var glimmerEngine = require('glimmer-engine/ember-cli-build')({ |
There was a problem hiding this comment.
We can probably just delete it right?
chadhietala
force-pushed
the
macros
branch
4 times, most recently
from
e0c1408
to
53e8493
Compare
This also changes the build to use the individual glimmer packages
| @@ -64,7 +71,7 @@ | |||
| "git-repo-info": "^1.1.4", | |||
| "git-repo-version": "^0.3.1", | |||
| "github": "^0.2.3", | |||
| "glimmer-engine": "^0.19.4", | |||
| "glimmer-engine": "^0.20.0", | |||
There was a problem hiding this comment.
Do we still need glimmer-engine?
| @@ -27,6 +27,11 @@ | |||
| "test:testem": "testem -f testem.dist.json" | |||
| }, | |||
| "dependencies": { | |||
| "@glimmer/compiler": "^0.21.1", | |||
| "@glimmer/node": "^0.21.1", | |||
| "@glimmer/reference": "^0.21.0", | |||
There was a problem hiding this comment.
Should these all be the same version?
There was a problem hiding this comment.
No I don't believe so. The way lerna publish works is that it looks if packages were effected in between the HEAD and the last tag if its not effected it will not publish a new version.
| @@ -35,12 +40,14 @@ | |||
| "ember-cli-test-info": "^1.0.0", | |||
| "ember-cli-valid-component-name": "^1.0.0", | |||
| "ember-cli-version-checker": "^1.1.7", | |||
| "handlebars": "~3", | |||
There was a problem hiding this comment.
Why does ember have a handlebars dep? Seems like this should be in Glimmer and handled upon its publishing, or am I missing something?
There was a problem hiding this comment.
@krisselden might want to chime in here. I don't remember why we did this.
| @@ -206,6 +205,11 @@ function createCurriedDefinition(definition, args) { | |||
| ); | |||
| } | |||
|
|
|||
| const EMPTY_BLOCKS = { | |||
There was a problem hiding this comment.
at least in dev mode, freezing it does make it a different "shape" polymorphic with the unfrozen versions, not sure if it is already polymorphic and empty and non empty should be both seen by the time it is JITed so may not matter.
| let { symbolTable } = builder; | ||
|
|
||
| let definition; | ||
| if (name.indexOf('-') > -1) { |
There was a problem hiding this comment.
Should we use the contains dash caching here?
| hash | ||
| } = BaselineSyntax.NestedBlock; | ||
|
|
||
| export function _inElementMacro(sexp, builder) { |
There was a problem hiding this comment.
Why is this moved into Ember itself? I thought it was baked into Glimmer?
There was a problem hiding this comment.
Seems like it was removed from Glimmer.
|
|
||
| export function registerSyntax(key, syntax) { |
There was a problem hiding this comment.
I would still like to export a mechanism to register macros. Without one we loose the ability to experiment with different syntaxes (e.g. ember-let).
It seems totally good to change it to a more generic thing like registerMacro, but we shouldn't remove the ability to customize/experiment.
There was a problem hiding this comment.
I added an API for this. Let me know what you think.
|
Awesome job here @chadhietala! |
This moves Ember over to the new Binary VM in glimmer (Requires this branch). Notable changes:
{{debugger}}is now implemented as a built-in Glimmer statement syntax due to the fact it needs access to theSymbolTableand other information from the VM. Also we now hard error if the debugger is passed an expression, from what I can tell this just failed silently is we did nothing with the positional or named params.refineStatementon theEnvironmentis no more in favor ofpopulateMacrosandblock|inlines.addMissing.