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

fix(scan): handle local scripts with lang=ts #5850

Merged
merged 3 commits into from
Dec 1, 2021

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Nov 26, 2021

Description

When <script setup lang="ts"> in Vue or <script lang="ts"> in Svelte is written (aka local scripts), esbuild's ts loader skips imports that looks unused. The normal scanner has a fix for it, but not for local scripts

Also adds missing tests for #5711. And added global serverLogs variable for testing.

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@bluwy bluwy added the p1-chore Doesn't change code behavior (priority) label Nov 26, 2021
@bluwy
Copy link
Member Author

bluwy commented Nov 26, 2021

Strange looks like the fix I implemented isn't actually working? 🤔 Marking as draft for now.

@bluwy bluwy marked this pull request as draft November 26, 2021 08:54
@bluwy bluwy changed the title chore: add vue dep scan test fix(scan): handle local lang ts scripts Nov 29, 2021
@bluwy bluwy changed the title fix(scan): handle local lang ts scripts fix(scan): handle local scripts with lang=ts Nov 29, 2021
@bluwy bluwy added p3-minor-bug An edge case that only affects very specific usage (priority) and removed p1-chore Doesn't change code behavior (priority) labels Nov 29, 2021
@bluwy bluwy marked this pull request as ready for review November 29, 2021 18:07
@bluwy
Copy link
Member Author

bluwy commented Nov 29, 2021

Found a bug while adding the new test, so now this PR is a bugfix instead of a chore. Ready for review.

@patak-dev
Copy link
Member

patak-dev commented Nov 29, 2021

Great catch @bluwy!

As a note for later, maybe importsNotUsedAsValues could be used to avoid the hack? It is going to be supported from esbuild 0.14. See PR targeting vite 2.8 #5861

@bluwy
Copy link
Member Author

bluwy commented Nov 30, 2021

Interesting idea! Yeah I think that could work too. We can try that when that's merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants