Skip to content

Conversation

@Stijn98s
Copy link
Contributor

Closes #297

@Stijn98s Stijn98s marked this pull request as ready for review November 14, 2020 20:11
@Stijn98s
Copy link
Contributor Author

@lmiller1990 what do you think about this fix?

@lmiller1990 lmiller1990 self-requested a review November 15, 2020 08:02
Copy link
Member

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Nice job, logic seems fine (bit dirty, but 🤷‍♂️ I don't see a cleaner way right now).

Can you address the one comment I left? Thanks!

tempOutput.match(/\}\(.*.?Vue\);/) &&
tempOutput.includes('vue-class-component')
) {
node.add(';exports.default = {...exports.default.__vccBase};')
Copy link
Member

Choose a reason for hiding this comment

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

I think we will now end up with 2x node.add(';exports.default = {...exports.default};') (added here and line 43).

Can you 1) check if that is the case and 2) make sure we only add it once? If we aren't careful, this can become really complex really fast!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

node now ends with

';exports.default = {...exports.default.__vccBase};',
';exports.default = {...exports.default, render};' 

or

';exports.default = {...exports.default.__vccBase};',
';exports.default = {...exports.default};' 

I think this is the desired behavoir because otherwise the code will be more complex.

Example:

if (
  tempOutput.match(/\}\(.*.?Vue\);/) &&
  tempOutput.includes('vue-class-component')
) {
if (tempOutput.includes('exports.render = render;')) {
    node.add(';exports.default = {...exports.default.__vccBase, render};')
  } else {
    node.add(';exports.default = {...exports.default.__vccBase};')
  }
} else {
  if (tempOutput.includes('exports.render = render;')) {
    node.add(';exports.default = {...exports.default, render};')
  } else {
    node.add(';exports.default = {...exports.default};')
  }
}

this will have an output of

';exports.default = {...exports.default.__vccBase, render};'

or

';exports.default = {...exports.default.__vccBase};'

Copy link
Member

Choose a reason for hiding this comment

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

Right, true, that is very complex. Fair enough.

I wonder if we should create a isClassComponent function 🤔

Anyway, this will do for now, I will test it out a bit and if it's all good do a release. thanks!

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.

3 participants