-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
Also hoist them above the `jsxWrapper`, if we’re going to create one.
If there is a `containingJSXElement`, we would have already searched for other JSX elements on it.
Also cleans up all the data we’re passing around. 😄
Ping @thejameskyle, I think it's ready for a code review now. |
I'll try to remember for later tonight |
Ping. 😃 |
|
||
file.path.unshiftContainer("body", helperAstFn(t, helperRef, dependencyRefs)); | ||
|
||
for (let dependency of undefinedDeps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this for...of
? This will depend on the polyfill
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
This looks really good. Sorry for taking so long to review it |
let key = null; | ||
let statics = []; | ||
|
||
for (let attribute of attributes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I remove this for ... of
, too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, any of them
Closes #8.
Code needs to be cleaned up a bit, and I need to rewrite the commit history. But, I'd rather get some input into how we can break this implementation before I spend time cleaning it up.