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.

Sign up for GitHub

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

Jump to bottom

Wrong assertion for local variable shadowing helper #17370

Closed
dguettler opened this issue Dec 14, 2018 · 3 comments · Fixed by #17399
Closed

Wrong assertion for local variable shadowing helper #17370

dguettler opened this issue Dec 14, 2018 · 3 comments · Fixed by #17399
Labels
Bug

Comments

@dguettler
Copy link

Affected: Ember 3.6
Related PR: #17132

Code causing an issue:

{{#with (changeset model EditSiteGroupValidation) as |changeset|}}
  {{site-group-form changeset=changeset
                    onSubmit=(route-action "update")
                    onCancel=(route-action "cancel" changeset)}}
{{/with}}

Error raised:

Cannot invoke the `changeset` helper because it was shadowed by a local variable (i.e. a block param) with the same name. Please rename the local variable to resolve the conflict.

Changing above code to:

{{#with (changeset model EditSiteGroupValidation) as |model|}}
  {{site-group-form changeset=model
                    onSubmit=(route-action "update")
                    onCancel=(route-action "cancel" model)}}
{{/with}}

passes compilation and does not raise an error.

Expected:
Original code should not raise an error as changeset is not invoked in the block.
@rwjblue
Copy link
Member

rwjblue commented Dec 14, 2018

Thanks for reporting this issue! I agree that the template snippet does not use the yielded block param in an invocation at all.

@chancancode - Would you mind double checking my logic here? I don’t think this case was intended to get the assertion, right?

@pixelhandler
Copy link
Contributor

pixelhandler commented Dec 14, 2018 •
edited
Loading

Well, I think it's a bad idea to use a block param and shadow a helper, I like the error. We ran into this and changed our block param variable name. I thought the error was helpful, but it was a little confusing at first, then I read Please rename the local variable to resolve the conflict. and it made more sense.

Perhaps the error needs to change to a warning?

The `changeset` helper is shadowed by a local variable (i.e. a block param) with the same name. Please rename the local variable to resolve the conflict.

@chancancode
Copy link
Member

@rwjblue yep this is a bug

wycats pushed a commit that referenced this issue Dec 20, 2018
@chancancode
[BUGFIX release] local variable shadowing assert
c142862 
Previously we considered the block params to shadow the params, hash,
attributes and named arguments on the same block/element, which was
incorrect.

Also fixes a bug where we incorrectly transformed mustaches in attribute
positions:

```hbs
{{#let ... as |foo|}}
  <div class={{foo bar}} />
{{/let}}
```

...became...

```hbs
{{#let ... as |foo|}}
  <div class={{component foo bar}} />
{{/let}}
```

This is clearly incorrect and has been fixed here as well.

Fixes #17370
@chancancode chancancode mentioned this issue Dec 20, 2018
Merged
wycats pushed a commit that referenced this issue Dec 20, 2018
@chancancode
[BUGFIX release] local variable shadowing assert
a442a8a 
Previously we considered the block params to shadow the params, hash,
attributes and named arguments on the same block/element, which was
incorrect.

For example:

```hbs
{{#let (concat "foo" "bar") as |concat|}}
  ...
{{/let}}
```

In this case the assertion code incorrectly believed the `concat` in
the sub-expression invocation was being shadowed. This is now fixed.

Also fixes a bug where we incorrectly transformed mustaches in attribute
positions:

```hbs
{{#let ... as |foo|}}
  <div class={{foo bar}} />
{{/let}}
```

...became...

```hbs
{{#let ... as |foo|}}
  <div class={{component foo bar}} />
{{/let}}
```

This is clearly incorrect and has been fixed here as well.

Fixes #17370
wycats pushed a commit that referenced this issue Dec 20, 2018
@chancancode
[BUGFIX release] local variable shadowing assert
d25463b 
Previously we considered the block params to shadow the params, hash,
attributes and named arguments on the same block/element, which was
incorrect.

For example:

```hbs
{{#let (concat "foo" "bar") as |concat|}}
  ...
{{/let}}
```

In this case the assertion code incorrectly believed the `concat` in
the sub-expression invocation was being shadowed. This is now fixed.

Also fixes a bug where we incorrectly transformed mustaches in attribute
positions:

```hbs
{{#let ... as |foo|}}
  <div class={{foo bar}} />
{{/let}}
```

...became...

```hbs
{{#let ... as |foo|}}
  <div class={{component foo bar}} />
{{/let}}
```

This is clearly incorrect and has been fixed here as well.

Fixes #17370
wycats pushed a commit that referenced this issue Dec 20, 2018
@chancancode
[BUGFIX release] local variable shadowing assert
030f373 
Previously we considered the block params to shadow the params, hash,
attributes and named arguments on the same block/element, which was
incorrect.

For example:

```hbs
{{#let (concat "foo" "bar") as |concat|}}
  ...
{{/let}}
```

In this case the assertion code incorrectly believed the `concat` in
the sub-expression invocation was being shadowed. This is now fixed.

Also fixes a bug where we incorrectly transformed mustaches in attribute
positions:

```hbs
{{#let ... as |foo|}}
  <div class={{foo bar}} />
{{/let}}
```

...became...

```hbs
{{#let ... as |foo|}}
  <div class={{component foo bar}} />
{{/let}}
```

This is clearly incorrect and has been fixed here as well.

Fixes #17370
wycats pushed a commit that referenced this issue Dec 20, 2018
@chancancode
[BUGFIX release] local variable shadowing assert
1343d29 
Previously we considered the block params to shadow the params, hash,
attributes and named arguments on the same block/element, which was
incorrect.

For example:

```hbs
{{#let (concat "foo" "bar") as |concat|}}
  ...
{{/let}}
```

In this case the assertion code incorrectly believed the `concat` in
the sub-expression invocation was being shadowed. This is now fixed.

Also fixes a bug where we incorrectly transformed mustaches in attribute
positions:

```hbs
{{#let ... as |foo|}}
  <div class={{foo bar}} />
{{/let}}
```

...became...

```hbs
{{#let ... as |foo|}}
  <div class={{component foo bar}} />
{{/let}}
```

This is clearly incorrect and has been fixed here as well.

Fixes #17370
kategengler pushed a commit that referenced this issue Dec 24, 2018
@chancancode @kategengler
[BUGFIX release] local variable shadowing assert
d56df77 
Previously we considered the block params to shadow the params, hash,
attributes and named arguments on the same block/element, which was
incorrect.

For example:

```hbs
{{#let (concat "foo" "bar") as |concat|}}
  ...
{{/let}}
```

In this case the assertion code incorrectly believed the `concat` in
the sub-expression invocation was being shadowed. This is now fixed.

Also fixes a bug where we incorrectly transformed mustaches in attribute
positions:

```hbs
{{#let ... as |foo|}}
  <div class={{foo bar}} />
{{/let}}
```

...became...

```hbs
{{#let ... as |foo|}}
  <div class={{component foo bar}} />
{{/let}}
```

This is clearly incorrect and has been fixed here as well.

Fixes #17370

(cherry picked from commit 1343d29)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[BUGFIX release] local variable shadowing assert emberjs/ember.js
4 participants
@dguettler @rwjblue @pixelhandler @chancancode