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.

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

Non-helper functions not deconflicted #388

Closed
Conduitry opened this issue Mar 17, 2017 · 5 comments · Fixed by #389
Closed

Non-helper functions not deconflicted #388

Conduitry opened this issue Mar 17, 2017 · 5 comments · Fixed by #389

Comments

@Conduitry
Copy link
Member

This is a bit of a weird case to handle, because while we do want to rename the applyComputations function if it conflicts with imports, we don't want to just use the existing helper(name) function, as that then adds the function to .uses and we'd try to bring in the shared function by that name, which doesn't exist.

Saving gists seems broken at the moment in the REPL, so here's a quick reproduction:

<script>
  import applyComputations from "./dummy";
  export default {
    computed: {
      a: b => b
    }
  }
</script>
@Conduitry
Copy link
Member Author

This is also going to happen with addCss:

<script>
  import addCss from "./dummy";
</script>
<style>
</style>

I don't think it can happen with anything else. Updating ticket description.

@Conduitry
Copy link
Member Author

Nope, also renderMainFragment.

@Conduitry Conduitry changed the title applyComputations not deconflicted Non-shared helper functions not deconflicted Mar 17, 2017
@Conduitry Conduitry changed the title Non-shared helper functions not deconflicted Non-helper functions not deconflicted Mar 18, 2017
@Conduitry
Copy link
Member Author

(Also addedCss.)

The easiest way I could think of to handle this is with an extra flag passed to the generator.helper method. I've pushed what I have but haven't submitted a PR. It looks like it works, and it passes all the existing unit tests, but could probably stand a couple new ones.

@Conduitry
Copy link
Member Author

Splitting off the alias generation into a separate method might be nicer than just adding a boolean argument to the existing method.

@Conduitry
Copy link
Member Author

PR #389 submitted, including unit test.

Rich-Harris added a commit that referenced this issue Mar 18, 2017
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 a pull request may close this issue.

1 participant