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: Use lower case filenames for types so they can be imported correctly on Linux #9827

Closed
wants to merge 1 commit into from

Conversation

Princesseuh
Copy link
Contributor

Description

Due to an issue in TypeScript (microsoft/TypeScript#45096), files using uppercased characters can't be imported through a triple slash directive types on Linux.

This is normally not an issue, because Vite refers to those types exclusively using path (since #4031), however it's an issue for third-party projects who needs to refers to those types using types. Example issue from the Astro repo: withastro/astro#4387

Additional context

This should be all, don't hesitate if there's anything I missed!


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.

@patak-dev
Copy link
Member

Thanks for the PR @Princesseuh!

The current file names are part of Vite's API so I'm trying to think how we can make this change without breaking downstream projects. See for example https://vitejs.dev/guide/api-plugin.html#client-server-communication

Maybe we could keep the previous files and point in them to the new ones? If there is a way to make this work, we could release this as a non-breaking change in 3.1 and state that the old files will be removed in Vite 4.

@patak-dev patak-dev requested a review from bluwy August 24, 2022 21:18
@patak-dev patak-dev added the p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) label Aug 24, 2022
@Princesseuh
Copy link
Contributor Author

Thanks for the PR @Princesseuh!

The current file names are part of Vite's API so I'm trying to think how we can make this change without breaking downstream projects. See for example https://vitejs.dev/guide/api-plugin.html#client-server-communication

Maybe we could keep the previous files and point in them to the new ones? If there is a way to make this work, we could release this as a non-breaking change in 3.1 and state that the old files will be removed in Vite 4.

While we can do that successfully (as far as I can tell) for most of the files, we can't do this for importMeta.d.ts because we can't export from it since it's a type augmentation. We also can't have the same file duplicated because then you get an error about a duplicate definition (so you'd get an error running tsc without skipLibCheck anyway)

I'm not too sure how to achieve this, hmm. I don't necessarily mind this being a breaking change, fwiw. We (Astro) and other frameworks can fairly easily duplicate the file manually in our repos for now as a workaround

@patak-dev
Copy link
Member

I'll add this PR to be discussed in a future team meeting. Maybe it is enough to do this change in a minor, but if not, we could do it in Vite 4 that isn't that far away (probably a few months from now)

@bluwy
Copy link
Member

bluwy commented Aug 26, 2022

We (Astro) and other frameworks can fairly easily duplicate the file manually in our repos for now as a workaround

I think this would be the way for now too. I also think that this is a breaking change that we should do for Vite 4.

Also looks like microsoft/TypeScript#45096 is marked in TS 4.8 milestone (though 4.8 is already released). Perhaps it might be resolved soon.

@patak-dev patak-dev added this to the 4.0 milestone Aug 26, 2022
@patak-dev
Copy link
Member

Ok, let's target v4. I removed it from the meeting discussion for now.

@patak-dev
Copy link
Member

@Princesseuh would you help to test @sapphi-red's #9966? Looks like we could resolve this issue without waiting until Vite 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants