Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove support for Partials #1547

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Remove support for Partials #1547

wants to merge 5 commits into from

Conversation

NullVoxPopuli
Copy link
Contributor

wip

@@ -52,16 +52,13 @@ class ScopeInspector {
let { scope, locals } = this;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we still generally treat the {{debugger}} statement as an "runtime eval", which is the same kind of thing as partial.

In order to support it, we have to keep around a lot more strings labels (the names of the variables) than we otherwise need to, just so when we "eval" code (in the case of {{debugger}}, calling the get("...") function in the console where "..." can be any local variables, named arguments, etc.

I think now that "implicit this" and partials are no longer supported, I think we should be able to accomplish the same by doing a bit more analysis at pre-compile time. We should be able to see all the in-scope local variables at that point, and we can internally rewrite:

{{#let this.foo as |foo|}}
  {{#let foo.bar as |bar|}} 
    {{!-- magically `get("foo")` and `get("bar")` has to work in the console --}}
    {{debugger}}
  {{/let}}
{{/let}}

into

{{#let this.foo as |foo|}}
  {{#let foo.bar as |bar|}} 
    {{debugger foo=foo bar=bar}}
  {{/let}}
{{/let}}

Basically we pass every known local variables (and there is no longer such thing as "unknown local variables" anymore) as named arguments, and that should give sufficient information for the {{debugger}} implementation to make get() work without any pervasive book-keeping everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants