kit
kit copied to clipboard
fix(enhanced-img): preserve ambient context for `*?enhanced` imports
Preserving the ambient context
#12224 changes the default export of *?enhanced modules from a value of type string into a value of type Picture (from vite-imagetools). However, this change caused the ambient.d.ts file to no longer be an ambient module. According to the TypeScript docs, top-level imports and exports automatically upgrade the ambient context into a module context. That means the declare module '*?enhanced' is no longer applied "ambiently", but now requires an explicit import somehow. This breaks svelte-check and other static analysis tooling because TypeScript would report that any module imported as *?enhanced have no existing type declarations.
The fix is quite simple: simply move the top-level import into the declare module syntax as shown in 4455725535f2ecf617029b83cbc6322c2fe677f2. This preserves the "ambient-ness" of the ambient.d.ts.
Ensuring that dependencies are present with pnpm
This PR also fixes a possible footgun when importing modules with pnpm. By default, pnpm does not hoist dependencies to the top level of the node_modules. The direct dependencies of each package is thus locally scoped to that package only. If a package does not declare the dependency, pnpm will not resolve it.
This is problematic for the types/ of @sveltejs/enhanced-img because it imports from vite and svelte. Previously, these were declared as devDependencies, which causes pnpm to prune them from module resolution.
This PR fixes this by upgrading vite and svelte as direct dependencies. Alternatively, this may be downgraded to peerDependencies instead. Let me know if this is preferable.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
- [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
- [ ] This message body should clearly illustrate what problems it solves.
- [ ] Ideally, include a test that fails without this PR but passes with it.
Tests
- [x] Run the tests with
pnpm testand lint the project withpnpm lintandpnpm check
Changesets
- [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.
Edits
- [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.
🦋 Changeset detected
Latest commit: 5dbbe0b807d6cf29bd23dca9cc63fecb01350cba
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @sveltejs/enhanced-img | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
@benmccann Heads up: CI lints are failing due to the outdated root lockfile. I have regenerated it using pnpm install. CI should pass now. Please don't mind the thousand-line diff. Thanks!
I have regenerated it using pnpm install. CI should pass now. Please don't mind the thousand-line diff.
I didn't want the thousand line diff, so fixed it and did a force push. That means you won't be able to make changes on your branch without deleting it and checking it out again, but hopefully that's fine as there probably aren't further changes needed at this point
I didn't want the thousand line diff, so fixed it and did a force push.
Thanks! This is super appreciated. The diffs look much cleaner now. 🚀
That means you won't be able to make changes on your branch without deleting it and checking it out again, but hopefully that's fine as there probably aren't further changes needed at this point.
No worries, nothing that a good ol' git fetch && git reset --hard origin/main can't fix. 😅
Hello there! Just checking up and bumping this PR. It would be really nice to have this patch upstream so that our CI need not depend on patched/overridden versions. Thanks! 🚀