-
Notifications
You must be signed in to change notification settings - Fork 2k
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
meta: enforce .ts
extension for relative import types
#5393
Conversation
Diff output filesNo diff |
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.
I strongly recommend to use the technically correct solution over a personal preference. The technically correct solution for libraries is to use the .js
extension for relative imports and the "module": "node16"
option in tsconfig.json
.
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.
I think most of typescript developers are used to always using .js
(because we had no choice until recently) although it's arguably a bit weird. But at least this PR makes it more consistent (relative import type
and import
both have to use .ts
now), so I'm fine with merging this PR. Should also be easy to change this in the future if it turns out problematic, or if someone has the time/motivation to make a PR that enforces .js
only with eslint rules.
wait - does this mean that because we spit out |
No, TS 5.0+ accepts resolving |
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 4.0.3 | @uppy/provider-views | 4.0.1 | | @uppy/companion | 5.0.5 | @uppy/status-bar | 4.0.2 | | @uppy/companion-client | 4.0.1 | @uppy/transloadit | 4.0.2 | | @uppy/core | 4.1.1 | @uppy/tus | 4.0.1 | | @uppy/dashboard | 4.0.3 | @uppy/utils | 6.0.2 | | @uppy/drag-drop | 4.0.2 | @uppy/vue | 2.0.1 | | @uppy/file-input | 4.0.1 | uppy | 4.1.1 | | @uppy/image-editor | 3.0.1 | | | - @uppy/transloadit: fix issue with `allowMultipleUploadBatches` (Mikael Finstad / #5400) - meta: Bump elliptic from 6.5.5 to 6.5.7 (dependabot[bot] / #5410) - meta: add back patch for `p-queue` (Antoine du Hamel / #5409) - @uppy/transloadit: fix many lurking `TypeError` (Mikael Finstad / #5399) - docs: improve `corsOrigins` documentation (Mikael Finstad / #5390) - docs: add `ViewEncapsulation` to Angular example (Aaron Russell / #5395) - @uppy/companion: fix code for custom providers (Mikael Finstad / #5398) - docs: add note about throwing in `cancelAll` and `destroy()` (Mikael Finstad / #5408) - meta: Bump docker/login-action from 3.2.0 to 3.3.0 (dependabot[bot] / #5372) - meta: Bump docker/setup-qemu-action from 3.1.0 to 3.2.0 (dependabot[bot] / #5370) - docs: make hosted Companion more clear (Merlijn Vos / #5394) - meta: Bump docker/build-push-action from 6.4.1 to 6.6.1 (dependabot[bot] / #5403) - meta: bump p-queue to latest, remove patch (Mikael Finstad / #5391) - meta: enforce `.ts` extension for relative import types (Antoine du Hamel / #5393) - @uppy/tus: Fix onShouldRetry type signature (Trent Nadeau / #5387) - @uppy/dashboard,@uppy/drag-drop,@uppy/file-input: Transform the `accept` prop into a string everywhere (Evgenia Karunus / #5380) - docs: fix getTemporarySecurityCredentials in aws-s3 (Merlijn Vos / #5363)
It makes more sense for
import
andimport type
to be consistent with those rules:.ts
or.tsx
file extension.lib/*.js
files, not thesrc/*.ts
orsrc/*.tsx
files.Using
.ts
inimport type
declarations means they are going to show up in.d.ts
files (which is a bit awkward, because in thelib/
folder there won't be a.ts
file but a.js
one), but we already have those (e.g.packages/@uppy/google-drive/lib/index.d.ts
hasexport { default } from './GoogleDrive.tsx'
), and TS 5.0+ handles it fine (it's going to resolve the import toindex.d.ts
).