natemoo-re / astro-icon

Inline and sprite-based SVGs in Astro made easy!
https://astroicon.dev
Other
996 stars 57 forks source link

v1.0 #111

Closed natemoo-re closed 6 months ago

natemoo-re commented 1 year ago

Finally, astro-icon@1.0.0 is here! This release provides a more flexible and reliable method of icon management, removes a number of redundant APIs, and now relies on Astro's official integration API.

Please see the Upgrade to astro-icon v1 guide for specific migration steps and the package README for more information.

MASSIVE thank you to @stramel for keeping this effort alive while I was too burned out to help.


Closes #87, closes #71, fixes #29, fixes #35, resolves #38, resolves #43, resolves #54, resolves #65, resolves #68, resolves #73

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: f5548cff1b13b867313d30b0dd69f716f65c689f

The changes in this PR will be included in the next version bump.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

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

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
api-astroicon ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 22, 2023 7:56pm
astroicon ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 22, 2023 7:56pm
nimser commented 1 year ago

No insights, but how does that solve #29 ? I'm still missing icons, e.g. bi:balloon-heart-fill.

vercel[bot] commented 1 year ago

Deployment failed with the following error:

Resource is limited - try again in 2 hours (more than 100, code: "api-deployments-free-per-day").
Lunarequest commented 1 year ago

hi so i tried using this with my project and discovered that this pr uses pnpm, yarn 2.x will not be able to install this package if it uses pnpm as yarn does not yet have a resolver for pnpm packages. see https://github.com/yarnpkg/berry/issues/3169 for more info

stramel commented 1 year ago

Thanks for the feedback @Lunarequest! We should hopefully have an alpha release here soon which will resolve the install problem for you.

Lunarequest commented 1 year ago

Thanks for the feedback @Lunarequest! We should hopefully have an alpha release here soon which will resolve the install problem for you.

thank you!

alexanderniebuhr commented 1 year ago

@stramel I was trying to install it via URL this https://gitpkg.now.sh/natemoo-re/astro-icon/packages/core?v1 While it does install, running astro dev would not work. Would be great if you can either release an preview version to npm or let me know how to install this branch and test it :)


Failed to resolve entry for package "astro-icon". The package may have incorrect main/module/exports specified in its package.json.
stramel commented 1 year ago

@stramel I was trying to install it via URL this gitpkg.now.sh/natemoo-re/astro-icon/packages/core?v1 While it does install, running astro dev would not work. Would be great if you can either release an preview version to npm or let me know how to install this branch and test it :)

Failed to resolve entry for package "astro-icon". The package may have incorrect main/module/exports specified in its package.json.

Yes, we are wanting to get a prerelease on NPM so that people can start giving it a shot without the workaround. I unfortunately don't have the ability to release. The v1 branch doesn't have the built code published on it so it won't work. However the code state is in nearly the same spot as my ms/v1 branch that I previously posted about using. So you should be able to use that branch still.

natemoo-re commented 1 year ago

Thanks for you patience everyone! Happy to say that astro-icon@next is published so you can try it out locally!

We have some docs and migration guides to write, so keep an eye out for those. Hoping to wrap this major upgrade up soon.

FineWolf commented 1 year ago

@natemoo-re Thanks for your hard work.

Just used astro-icon@next on two production websites, and had no issues. Both leveraged iconify-json packs and local icons.

alexanderniebuhr commented 1 year ago

Used it and figured it added 700kb to the bundled output.. This will break limits for the 1MB in Cloudflare e.g. Does it not tree-shake unused icons from iconify? :/

stramel commented 1 year ago

@alexanderniebuhr oof... That doesn't sound correct. Can you provide any details or links to the different build before and after? Would love to help resolve this. We actually are deduping icons now so it should result in a much smaller build.

alexanderniebuhr commented 1 year ago

Can you provide any details or links to the different build before and after?

https://gist.github.com/alexanderniebuhr/14603b653ceeb3be2a72dfcc482903e8

Please let me know if you need anything else.

Actually I think it does not really work well for Iconify as expected (further analysis shows), I would have to define each Icon in my config separately, like so:

icon({
    include: {
        carbon: ['3d-cursor'],
        charm: ['anchor', 'flame'],
    },
}),

This would keep the bundle size low, however it should work with [*], and tree-shacking to only bundle svg data of used icons.

stramel commented 1 year ago

Thank you for the details! I think I understand the issue. I will work on a resolution for this.

stramel commented 1 year ago

@alexanderniebuhr Confirmed the issue lies within the SSR while specifying '*'. Working on a solution that will remove this DX and bundling issue.

alexanderniebuhr commented 11 months ago

Working on a solution that will remove this DX and bundling issue.

Any update, can I test another version?

FDiskas commented 9 months ago

@natemoo-re - everything is ok?

stramel commented 8 months ago

@natemoo-re - everything is ok?

Sorry for the absence on here. We're currently not happy with the SSR DX story here. We're looking into a better fix.

In the meantime, @natemoo-re proposed core integration with icons within Astro. We will likely need to revist the v1 version once the direction has been decided upon.

natemoo-re commented 6 months ago

OK the issues here are stacking up, I know folks are waiting for fixes.

I'm definitely not happy with the SSR story at the moment, but including svgo at runtime seems to be the cause of the vast majority of problems with the current approach. This version at least solves that.

I think I might have an idea for how to move forward here while still supporting the remote icon service, but refactoring the service to be self-updating so it doesn't require as much maintenance from my end. The dynamic nature of this package is a huge part of the appeal. Ideally, SSR users could still use a string and hit the remote service, but if latency is a critical concern for them, they can switch to the local approach.

At the same time, I have proposed for SVG support to be built into Astro and that seems to be something everyone is interested in. But in the meantime I should get this v1.0 out. Hopefully it informs Astro's native approach.