Reuse removed vars in mangler #395
Merged
Conversation
This file contains 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
boopathi
added
enhancement
Tag: New Feature
|
@boopathi this looks great. The only thing I don't like is the "tracker" name since it doesn't really explain what it is or what it tracks and it might make reading code that uses it harder. Wouldn't "Scope" or "ScopeChain" be a better name? |
|
It is not type Tracker {
references: Map<Scope, CountedSet<String> >
bindings: Map<Scope, Map<String, Binding> >
}It counts references and maintains bindings. Maybe call it |
boopathi
force-pushed
the
reuse-vars-4
branch
from
eedcf45
to
33cef0d
Compare
|
Sure, that would be better.
…On Thursday, February 2, 2017, Boopathi Rajaa ***@***.***> wrote:
It is not Scope. It is basically -
type Tracker {
references: Map<Scope, CountedSet<String> >
bindings: Map<Scope, Map<String, Binding> >
}
It counts references and maintains bindings.
Maybe call it ScopeTracker ?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#395 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAABf1LiX5MEHBmmz-Wsom6fzPRVQ7f5ks5rYb03gaJpZM4LyGJw>
.
|
boopathi
force-pushed
the
reuse-vars-4
branch
2 times, most recently
from
cf98f73
to
f2d24f7
Compare
boopathi
force-pushed
the
reuse-vars-4
branch
5 times, most recently
from
033a3db
to
55bee2a
Compare
Merged
boopathi
force-pushed
the
reuse-vars-4
branch
2 times, most recently
from
0a36f1c
to
8f96264
Compare
Codecov Report
@@ Coverage Diff @@
## master #395 +/- ##
==========================================
- Coverage 83.26% 82.51% -0.76%
==========================================
Files 34 38 +4
Lines 2539 2693 +154
Branches 908 943 +35
==========================================
+ Hits 2114 2222 +108
- Misses 254 291 +37
- Partials 171 180 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #395 +/- ##
==========================================
- Coverage 83.26% 82.94% -0.32%
==========================================
Files 34 40 +6
Lines 2539 2715 +176
Branches 908 949 +41
==========================================
+ Hits 2114 2252 +138
- Misses 254 279 +25
- Partials 171 184 +13
Continue to review full report at Codecov.
|
ResetNext identifier only when reuse is true Fix tests - add keepClassName Reuse vars as default I dont know why it works - #326 Extract tracker to a separate file, Add topLevel Option
| if ( | ||
| path.parentPath.isMemberExpression({ property: node }) || | ||
| path.parentPath.isObjectProperty({ key: node }) | ||
| path.parentPath.isExportSpecifier() && |
There was a problem hiding this comment.
what about import specifiers? are they safe?
| let parent = scope; | ||
| do { | ||
| this.references.get(parent).add(name); | ||
| if (!binding) throw new Error("How did global come here"); |
There was a problem hiding this comment.
rename the error message properly?
vigneshshanmugam
approved these changes
May 22, 2017
1 task
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.
Duplicate of #284. Rebased with current master.
Changes added to master / Issues reported for master and propagated to this branch: