Handle repeated bindExpression and bindImport calls#10
Closed
dfreeman wants to merge 2 commits intoemberjs:jsutilsfrom
Closed
Handle repeated bindExpression and bindImport calls#10dfreeman wants to merge 2 commits intoemberjs:jsutilsfrom
bindExpression and bindImport calls#10dfreeman wants to merge 2 commits intoemberjs:jsutilsfrom
Conversation
Contributor
|
Apologies, but I've made this confusing by having two PRs going at once. I think these bugs might be fixed already in #9 |
Contributor
|
I will consolidate into one branch that is the 2.0 release candidate. |
Contributor
Author
|
Ah, I didn't realize further changes to The duplicate |
Contributor
|
Rebased and merged into jsutils. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@ef4 I've been spiking out a version of
ember-css-modulesthat uses the alphaJSUtilsfunctionality to transformlocal-classattributes into import references and found that repeated calls tobindExpressionandbindImportcan result in erroneous or redundant code.I've taken a pass at handling these cases more gracefully, but I'm not 100% sure of my mental model for the relationship between our
localsarray and what's ultimately produced asscope, so if I've got a bad assumption here I'm happy to make changes.bindExpressionIf I call
bindExpression('2 + 2', target, { nameHint: 'two' })twice in the same template, this would wind up emitting code like this, which throws aSyntaxErrordue to the duplicate declaration:This PR updates
bindExpressionto ensure it accounts for locals we introduced ourselves when looking for a fresh name.bindImportIf I call
bindImport('my-library', 'default', target, { nameHint: 'two' })twice in the same template, the code produced isn't incorrect, but it's a bit weird. In the tests, you'll see output like this, withtwoappearing twice inlocals:This PR updates
bindImportto reuse the existing identifier if possible, ensuring that we still alias it if necessary due to other elements in template scope.