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.

Sign up for GitHub

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

Jump to bottom

feat!: Add support for Svelte 4 #297

Merged
merged 15 commits into from
Sep 2, 2023
Merged

feat!: Add support for Svelte 4 #297

merged 15 commits into from
Sep 2, 2023

Conversation

userquin
Copy link
Member

closes #295

GauBen
GauBen approved these changes Aug 18, 2023
View reviewed changes
Copy link

@GauBen GauBen left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I've been using these definitions for some time and so far so good

@userquin userquin changed the title feat: virtual dts for Svelte 4 feat!: virtual dts for Svelte 4 Sep 1, 2023
@userquin userquin marked this pull request as ready for review September 1, 2023 20:15
@userquin userquin changed the title feat!: virtual dts for Svelte 4 feat!: Add support for Svelte 4 Sep 1, 2023
benmccann
benmccann approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

I wonder if we should also export types/svelte as an alias for whatever the newest version of svelte is

@userquin userquin marked this pull request as draft September 1, 2023 20:28
@userquin userquin marked this pull request as ready for review September 1, 2023 20:49
@userquin userquin requested a review from antfu September 1, 2023 20:51
userquin
userquin commented Sep 1, 2023
View reviewed changes
types/svelte.d.ts
import { SvelteComponentTyped } from 'svelte'
export default class extends SvelteComponentTyped<svelte.JSX.IntrinsicElements['svg']> {}
}
import './svelte4.d.ts'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

node module resolution needs the file to be there

userquin
userquin commented Sep 1, 2023
View reviewed changes
Copy link
Member Author

@userquin userquin left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ben remove the comment and I just send another comment (creating a new review)

benmccann
benmccann approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@benmccann benmccann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry. Your response to my earlier comment wasn't showing up, so I deleted my comment when I realized it was wrong

benmccann
benmccann reviewed Sep 2, 2023
View reviewed changes
README.md Outdated
```ts
import 'unplugin-icons/types/svelte'
```js
/// <reference types="unplugin-icons/types/svelte" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be import 'unplugin-icons/types/svelte'; here and in the example. I'm not much of a TypeScript expert, but my understanding is the the default app.d.ts in SvelteKit projects is written in such a way as to work with and expect import to be used: sveltejs/kit#8276

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, also included global in app.d.ts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VSCode resolves virtuals properly, but missing types when using IntelliJ, maybe Svelte Plugin problem (the file is resolved with crtl + mouse click):

imagen

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new src/vite-env.d.ts file with triple slash references works in VSCode and IntelliJ (Node 18, TypeScript 5).

I guess IntelliJ in monorepo with Vue and Svelte not working properly (screenshot using kit project), there are a lot of options in IntelliJ and TypeScript (Vue: Vola or TS, TS Lint, Svelte, TS itself...)

image

@antfu antfu merged commit 263a9ef into main Sep 2, 2023
1 check passed
@antfu antfu deleted the userquin/feat-add-svelte-4-dts branch September 2, 2023 21:02
qwerty287 referenced this pull request in woodpecker-ci/woodpecker Sep 8, 2023
@renovate @qwerty287
chore(deps): update dependency unplugin-icons to ^0.17.0 (#2394)
d44b2a3 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [unplugin-icons](https://togithub.com/antfu/unplugin-icons) |
[`^0.16.3` ->
`^0.17.0`](https://renovatebot.com/diffs/npm/unplugin-icons/0.16.3/0.17.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/unplugin-icons/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/unplugin-icons/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/unplugin-icons/0.16.3/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/unplugin-icons/0.16.3/0.17.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### ⚠ Dependency Lookup Warnings ⚠

Warnings were logged while processing this repo. Please check the
Dependency Dashboard for more information.

---

### Release Notes

<details>
<summary>antfu/unplugin-icons (unplugin-icons)</summary>

###
[`v0.17.0`](https://togithub.com/unplugin/unplugin-icons/releases/tag/v0.17.0)

[Compare
Source](https://togithub.com/antfu/unplugin-icons/compare/v0.16.6...v0.17.0)

#####    🚨 Breaking Changes

- Add support for Svelte 4  -  by
[@&#8203;userquin](https://togithub.com/userquin) in
[https://github.com/unplugin/unplugin-icons/issues/297](https://togithub.com/unplugin/unplugin-icons/issues/297)
[<samp>(263a9)</samp>](https://togithub.com/unplugin/unplugin-icons/commit/263a9ef)

#####     [View changes on
GitHub](https://togithub.com/unplugin/unplugin-icons/compare/v0.16.6...v0.17.0)

###
[`v0.16.6`](https://togithub.com/unplugin/unplugin-icons/releases/tag/v0.16.6)

[Compare
Source](https://togithub.com/antfu/unplugin-icons/compare/v0.16.5...v0.16.6)

#####    🐞 Bug Fixes

- **compiler-solid**: Props replacement not working with multiline
starttags  -  by
[@&#8203;NathanHuisman](https://togithub.com/NathanHuisman) in
[https://github.com/antfu/unplugin-icons/issues/301](https://togithub.com/antfu/unplugin-icons/issues/301)
[<samp>(5668f)</samp>](https://togithub.com/antfu/unplugin-icons/commit/5668fbb)
- **types**: Fix type errors  -  by
[@&#8203;zyyv](https://togithub.com/zyyv) in
[https://github.com/antfu/unplugin-icons/issues/299](https://togithub.com/antfu/unplugin-icons/issues/299)
[<samp>(79412)</samp>](https://togithub.com/antfu/unplugin-icons/commit/7941238)

#####     [View changes on
GitHub](https://togithub.com/antfu/unplugin-icons/compare/v0.16.5...v0.16.6)

###
[`v0.16.5`](https://togithub.com/unplugin/unplugin-icons/releases/tag/v0.16.5)

[Compare
Source](https://togithub.com/antfu/unplugin-icons/compare/v0.16.4...v0.16.5)

#####    🐞 Bug Fixes

- Explicit .cjs and .mjs extension  -  by
[@&#8203;antfu](https://togithub.com/antfu)
[<samp>(45bc0)</samp>](https://togithub.com/antfu/unplugin-icons/commit/45bc00d)

#####     [View changes on
GitHub](https://togithub.com/antfu/unplugin-icons/compare/v0.16.4...v0.16.5)

###
[`v0.16.4`](https://togithub.com/unplugin/unplugin-icons/releases/tag/v0.16.4)

[Compare
Source](https://togithub.com/antfu/unplugin-icons/compare/v0.16.3...v0.16.4)

#####    🐞 Bug Fixes

- Update qwik support  -  by
[@&#8203;michaelhthomas](https://togithub.com/michaelhthomas) in
[https://github.com/antfu/unplugin-icons/issues/291](https://togithub.com/antfu/unplugin-icons/issues/291)
[<samp>(d3a85)</samp>](https://togithub.com/antfu/unplugin-icons/commit/d3a852f)

#####     [View changes on
GitHub](https://togithub.com/antfu/unplugin-icons/compare/v0.16.3...v0.16.4)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/woodpecker-ci/woodpecker).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi44My4wIiwidXBkYXRlZEluVmVyIjoiMzYuODMuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benmccann benmccann benmccann approved these changes

@GauBen GauBen GauBen approved these changes

@antfu antfu Awaiting requested review from antfu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Svelte 4 support
4 participants
@userquin @benmccann @GauBen @antfu