iconify / icon-sets

150+ open source icon sets. Icons are validated, cleaned up, optimised, ready to render as SVG. Updated automatically 3 times a week.
https://icon-sets.iconify.design/
608 stars 58 forks source link

Publish sub packages #15

Closed antfu closed 2 years ago

antfu commented 3 years ago

Hey again, I been using this package for a while and it's truly amazing!!

I made vite-plugin-icons to bundle icons on-demand and it's by far my best icons solution.

However the current concern is that when using it, you will need to install the entire @iconify/json (~120MB) to use only a few of them. If would be great if we could publish each collections as standalone packages, for example: @iconify/json-carbon, so the build tool could search for it on-demand.

If you like this idea, I am happy to send a PR :)

cyberalien commented 3 years ago

That makes sense.

Though I'm thinking if there is a limit on packages one developer can publish on NPM. Currently there is JSON package + 2 packages per icon set (one with ES exports, one with CommonJS exports), so total there are over 200 packages.

Maybe adding full JSON file to icon packages would be a better solution. But then it would make sense to add it as JavaScript file with export, similar to individual icon files, not JSON.

Need to think about it, but it can and will be done.

userquin commented 2 years ago

@antfu @cyberalien maybe we can include on next version

cyberalien commented 2 years ago

Yes, it is coming soon. I was doing some big refactoring in scan script to make it possible, should be ready next week.

cyberalien commented 2 years ago

This is how upcoming icon packages will be structured.

Each icon set will be published:

Each icon set will contain (all types are from @iconify/types package):

Example of package.json:

{
    "name": "@iconify-json/mdi",
    "version": "1.0.0",
    "iconSet": {
        "info": "info.json",
        "icons": "icons.json",
        "metadata": "metadata.json"
    }
}

Some things that will need testing: if main field of package.json can be pointed to json file, it will point to icons.json. I'll see if its possible to add .d.ts files for json files, if it is, package will also contains types and use @iconify/types as dependency.

What do you think about this?

antfu commented 2 years ago

That sounds great!

A few thoughts

{
  "main": "index.cjs",
  "module": "index.mjs",
  "types": "index.d.ts",
  "exports": {
    "./*": "./*",
    ".": {
      "require": "index.cjs",
      "import": "index.mjs"
    }
  }
}
// index.cjs
module.exports = require('./info.json')
// index.mjs
export { default as default } from './info.json'
// index.d.ts
import * from "@iconify/types"
userquin commented 2 years ago

@cyberalien @antfu so here the target is to have multiple packages, one for each collection? Maybe using optional deps on the main, providing also the all variant to provide current version.

Since I'm trying to switch to pnpm workspaces to include some example projects, I can create a script to create each icon set workspace.

Then, we'll need to switch the locate method to find collections from dependencies.

@antfu is that what you are suggesting?

@cyberalien is there some place where we can see the changes you are including?

cyberalien commented 2 years ago

Using workspaces is not a good idea. Currently script parses all icon sets, so it would work, but eventually I want to have better integration that updates icon sets almost instantly after author pushes update to icon set's repo. That means each icon set will have its own update process and multiple updates for different icon sets can be ran independently at the same time.

So goal is completely opposite to workspaces.

Functions like locate aren't needed. There is nothing to locate, each package contains only one icon set. To import it, just import package:

import { icons } from '@iconify-json/mdi';
userquin commented 2 years ago

Using workspaces is not a good idea. Currently script parses all icon sets, so it would work, but eventually I want to have better integration that updates icon sets almost instantly after author pushes update to icon set's repo. That means each icon set will have its own update process and multiple updates for different icon sets can be ran independently at the same time.

So goal is completely opposite to workspaces.

Functions like locate aren't needed. There is nothing to locate, each package contains only one icon set. To import it, just import package:

import { icons } from '@iconify-json/mdi';

The workspace will be only a trick to mantain here all packages together. I'll provide a ts script to generate them (only used on build time once before building json and icon-set-resolver modules and publishing), so all code base will be kept here using the json files. This new script will get each json file and create its own package inside its corresponding icon-sets/<prefix-name> workspace (published with @iconify-json/<prefix.name>): see https://pnpm.io/es/workspaces#publishing-workspace-packages.

We'll have 2 variant, @iconify/json (the current version on next PR) and @iconify/icon-set-resolver.

Then, locate on @iconify/icon-set-resolver will resolve looking from dependencies instead current version of next (I think it can be done using dynamic import).

On target project, adding @iconify-json/icon-set-resolver and the icon sets to be used: @iconify-json/mdi, @iconify-json/fa... should resolve the icon set provided with the prefix name, using another resolution type (using dynamic import).

export const lookupCollection = async(name: string): Promise<IconifyJSON> => {
  return await import(`@iconify-json/${name}`)
}

The resolve function should be useful on other builders like unplugin-icons, where the icons are resolved dynamically and so we cannot use your approach there.

It is just an idea, but if you prefer to keep separate ok, I'll keep as it is right now.

/cc @antfu I think this approach should work...

cyberalien commented 2 years ago

Published @iconify-json/carbon for testing.

Will be testing it to make sure it works correctly with various bundlers and stuff, then will do the same for everything else and will move it to automatic update.

userquin commented 2 years ago

Published @iconify-json/carbon for testing.

Will be testing it to make sure it works correctly with various bundlers and stuff, then will do the same for everything else and will move it to automatic update.

One last question, can we use here @iconify/types (adding the required types/interfaces) on the next PR version? I use it to create the types/interfaces for the current next version (I see that it is being used on @iconify-json/carbon).

cyberalien commented 2 years ago

Yes, @iconify/types is a stable package that should be used wherever possible. It was made to make sure various packages provide consistent data.

For next version I'll also be changing info in collections.json, which will switch to correct IconifyInfo type instead of current mess.

userquin commented 2 years ago

I'm doing some tests and seems to work on cjs and also on esm.

For example to test @iconify-json/carbon I just add it as dev dependency and returning the info entry:

imagen

imagen

imagen

imagen

userquin commented 2 years ago

Yes, @iconify/types is a stable package that should be used wherever possible. It was made to make sure various packages provide consistent data.

For next version I'll also be changing info in collections.json, which will switch to correct IconifyInfo type instead of current mess.

I'll review the current types and remove duplicated from next PR updating the types on this new @iconify/icon-set-resolver.

cyberalien commented 2 years ago

Thank you!

userquin commented 2 years ago

@cyberalien so, right now you'll keep the icons on your own repo? I will just update the types (you have some typos) and not as jsdocs but I can live with that.

If you want, you can take a look at the next PR to modify @iconify/types (I think both have the same order, I just copy/paste and then update jsdocs and correct some wording), for example, my version of IconifyDimensions:

/**
 * Icon dimensions.
 *
 * Used in:
 *  icon (as is)
 *  alias (overwrite icon's properties)
 *  root of JSON file (default values)
 */
export interface IconifyDimensions {
  /**
   * Left position of viewBox.
   *
   * @default 0
   */
  left?: number
  /**
   * Top position of viewBox.
   *
   * @default 0
   */
  top?: number
  /**
   * Width of viewBox.
   *
   * @default 16
   */
  width?: number
  /**
   * height of viewBox.
   *
   * @default 16
   */
  height?: number
}

while on @iconify/types you have (see the name IconifyDimenisons should be IconifyDimensions):

/**
 * Icon dimensions.
 *
 * Used in:
 *  icon (as is)
 *  alias (overwrite icon's properties)
 *  root of JSON file (default values)
 */
export interface IconifyDimenisons {
    // Left position of viewBox.
    // Defaults to 0.
    left?: number;

    // Top position of viewBox.
    // Defaults to 0.
    top?: number;

    // Width of viewBox.
    // Defaults to 16.
    width?: number;

    // Height of viewBox.
    // Defaults to 16.
    height?: number;
}
userquin commented 2 years ago

@cyberalien Should I do a test to include the icon sets as pnpm workspaces?

cyberalien commented 2 years ago

Icon set is not in any repo, its currently dynamically generated from this repo and published directly to NPM.

Types package is in this repo: https://github.com/iconify/iconify (as well as utils package).

I'm not sure about workspaces. Need to think about it, but currently don't see any upsides of using it for this repo. For repos with interconnected packages, I do see a point, such as iconify/iconify repo I've mentioned above, but for this repo I'm not sure what would be the point. Probably I'm missing something and just need to see how it works, so if you have time, please do show an example.

userquin commented 2 years ago

Icon set is not in any repo, its currently dynamically generated from this repo and published directly to NPM.

Types package is in this repo: https://github.com/iconify/iconify (as well as utils package).

I'm not sure about workspaces. Need to think about it, but currently don't see any upsides of using it for this repo. For repos with interconnected packages, I do see a point, such as iconify/iconify repo I've mentioned above, but for this repo I'm not sure what would be the point. Probably I'm missing something and just need to see how it works, so if you have time, please do show an example.

Ok, I'll try to generate the package for carbon icon set.

So, If I understand you, you create manually the @iconify-json/carbon? I mean, the only place where I can find @iconify-json/carbon is on NPM, but you will need to create its package to publish it (you need some process to create it: please share some hint so I can go to the solution directly). My solution is to add the package on this repo, and then publish it from here, you will also have some tools to update/override a custom one or just regenerate/override all at once.

The only thing I do not control yet is how to publish them all or just one or a set, I thing @antfu can tell us some hint... ;)

I'll try to just replicate the @iconify-json/carbon here using the info from collections.json and carbon.json file with a custom ts script (and maybe with a small cli only for development).

cyberalien commented 2 years ago

There is no need to use GitHub or any other service to publish packages. NPM is not linked to GitHub and packages do not require a repository.

userquin commented 2 years ago

@cyberalien Anyway, I'll create a new branch on my repo without icon sets and using pnpm workspaces (I need to add first 2 project examples, one for cjs and another for esm via "type": "module"), the names can be later changed and so you can take a look (maybe later we can move internal-types, json and icon-set-resolver inside packages directory).

I'm using types/interfaces from @iconify/types and I only had have to add these 2 new types (all old types/interfaces have been removed):

// icon-set-resolver/src/index.ts
import { IconifyInfo, IconifyJSON, IconifyMetaData, IconifyChars } from '@iconify/internal-types'

export type IconSetInfo = {
  info: IconifyInfo
  icons: IconifyJSON
  metadata: IconifyMetaData
  chars: IconifyChars
}

/**
 * Get a collection.
 *
 * @param {string} name The name of the collection
 * @return {Promise<IconifyJSON>}
 */
export const lookupCollection = async(name: string): Promise<IconSetInfo> => {
  return await import(`@iconify-json/${name}`)
}
// json/src/index.ts
/**
 * Collection info map
 */
export type IconifyMetaDataCollection = {
  [prefix: string]: IconifyMetaData
}

/**
 * Get list of collections info.
 *
 * @return {Promise<IconifyMetaDataCollection>}
 */
export const lookupCollections = async(): Promise<IconifyMetaDataCollection> => {
  return JSON.parse(await fs.readFile(join(dir, './collections.json'), 'utf8'))
}
cyberalien commented 2 years ago

Oh, now I see why you suggested workspaces. This package would contain only collections.json, not entire icon set, then resolver would load individual icon set package? Then actual data will be in individual repos, which will need to be on GitHub instead of current huge repo and workspaces will make sense.

Is that correct?

userquin commented 2 years ago

sense

yes, that's correct: instead having only the process, the repo will have in separate workspaces each icon-set, that can be generated, so we have all on the repo, don't need to publish to test it:

We will have 2 new workspaces under examples: cjs-test and esm-test to test json and icon-set-resolver.

Finally we will end having N workspaces inside icon-sets directory, one for each json file with its own package.json

userquin commented 2 years ago

Instead having an individual repo for each icon-set, we'll have 1 directory/workspace here inside icon-sets directory.

userquin commented 2 years ago

On icon-sets, each directory/workspace will contain the equivalent to @iconify-json/carbon:

imagen

cyberalien commented 2 years ago

This is a really cool idea. I love it.

There is an issue of content duplication. With this structure, individual icon sets cannot be in repo because it will duplicate content, which is already rather big. So it will have to be generated and published by script. So that part can be moved from internal script that updates packages to this repo. I think that can be done.... need to think about it.

userquin commented 2 years ago

You can generate the icon sets or not, it will be under your control, and so we can just create the repo, do some tests and we're done, you don't need to publish the icon set to test it.

Maybe we can have only the most popular like mdi and fa for example... we may generate only a small subset to do internal tests or for example when a new one added (we can also exclude from git, and so, ppl can have each own subset generated on local)...

cyberalien commented 2 years ago

By "need to think about it" I meant implementation :)

With this structure I think an entire part of publishing icon sets can be moved here and it can use GitHub actions to automate publishing. Then current update script can update icon sets in this repo, then GitHub will run action to publish individual packages. Also same can be applied to @iconify-icons/* packages. This would also make it easy for third party developers to generate various packages from their own icon sets.

userquin commented 2 years ago

With vite this will be simple, just a virtual module to transform so we only need to do the request, then the icon-set can be generated on the fly...

userquin commented 2 years ago

Also with rollup based bundler...

userquin commented 2 years ago

You can see an example on vite-plugin-pwa repo, we have precompiled mjs modules on src/client/build directory, then on the virtual module we just replace some info for the service worker, here we'll be able to create the entire module in a similar way.

userquin commented 2 years ago

Maybe we can do it with unplugin, that is, create a resolver for each bundler (vite, rollup, webpack...), @antfu has a lot of experience on this...

We have all the info on json files, maybe I can include an example and the virtual module for vite, just to show you some magic.

userquin commented 2 years ago

With this approach, you don't need to publish on npm the icon sets... I think this approach will feat your expectations, no duplicate code ofc, just include some pipeline in the process.

userquin commented 2 years ago

@cyberalien I have the new branch here https://github.com/userquin/collections-json/tree/feat/esm-support-icon-set pushed to my repo (will not have PR, maybe can replace actual PR, just take a look). I've included 2 examples, one for module and another for cjs.

Remember, this branch is using pnpm workspaces so you will need to add pnpm globally.

About the generator, I'll begin later, I need to eat something. I'll try to add the carbon version only.

Also created a vite project on local (not in the PR) exposing the info via virtual module, something like this:

<script setup lang="ts">
import carbon from 'virtual/icon-set/carbon'
</script>
<template>
  <pre>{{ carbon }}</pre>
</template>

I need to test the new functionality from @iconify/json (the old version) and test it with type module.

If you want, I can upload the ZIP here or where you can download it.

This virtual exports the info via dynamic import (the virtual module isn't exactly the unplugin-icons virtual module, only using some logic: we can expose the rest of the info, also icons, later I'll do a test to show them on the app):

import { defineConfig } from 'vite'
import Vue from '@vitejs/plugin-vue'
import { lookupCollection } from '@iconify/icon-set-resolver'

const URL_PREFIXES = ['virtual/icon-set/']
const iconPathRE = new RegExp(`${URL_PREFIXES.map(v => `^${v}`).join('|')}`)

function isIconPath(path: string) {
  return iconPathRE.test(path)
}

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [
    Vue(),
    {
      name: "custom-icon-set",
      enforce: 'pre',
      resolveId(id) {
        if (isIconPath(id)) {
          return id
        }
        return null
      },
      async load(id) {
        if (isIconPath(id)) {
          const { info }  = await lookupCollection(id.split('/')[2])
          return `export default ${JSON.stringify(info)}`
        }

        return null
      },
    }
  ]
})
userquin commented 2 years ago

Can also be loaded on demand with dynamic import also on the client side, this is the built version of the app not the dev.

imagen

userquin commented 2 years ago

If you want also the vite project (also using pnpm): https://wormhole.app/6RMWO#7uY18XpVlz27b7H_9Q3waA

EDIT: 30MB of zip file, it includes the old renovate version so inside we have all json files.

cyberalien commented 2 years ago

unplugin usage is really cool, but point of this GitHub issue is to make packages smaller, so client doesn't need to download all icon sets. So these small packages do need to be published on NPM.

Packages for individual icons though would have been great with unplugin, but I think time for that has passed because packages are already on NPM and they do get about 1k weekly downloads each. It might be a good idea to retire them later and switch to unplugin to generate individual icon files.

userquin commented 2 years ago

I need to fix some types...

cyberalien commented 2 years ago

Published all @iconify-json/* packages, added it to update script to automatically update packages.

userquin commented 2 years ago

@cyberalien https://github.com/antfu/unplugin-icons/pull/71

antfu commented 2 years ago

Thanks for all the awesome work! Two questions:

cyberalien commented 2 years ago

Thanks!

  • Will @iconify/json keep spinning around for a while or the new updates will all goes to @iconify-json/*?

Both will be up to date.

  • Do we have a @iconify/index package that lists all the support collections? (so in unplugin-icons we could read from it, and prompt for auto-install if the package isn't found).

Good point. I'll create a package for it.

userquin commented 2 years ago

@cyberalien @antfu amazing integration

userquin commented 2 years ago

@cyberalien I'll include on pending PR a set of new packages to allow you to test @iconify-json/* before publishing. It will help you for new icon sets addons or adding icons on existing ones, and you will be able to test on local before include them on the core.

I'll need to generate the ts module for the @iconify-json/* generation, so be patient.

Don't merge the PR #18 yet or move it to draft if you want.

Maybe I can "clone" this app as base for your tests: https://icones.js.org/.

@antfu it is a public repo? if so, where can I find it?

userquin commented 2 years ago

ok I don't see the github link...

antfu commented 2 years ago

Integration it to unplugin, but faced some issues. The viewBox generated from the new format it incorrect (always 0 0 16 16)

image

I tracked down and found that because the collection info (defaults) is not passed to getIconData. Is this a bug or we are doing anything wrong?

import mdi from '@iconify-json/mdi'
import { getIconData } from '@iconify/utils/lib/icon-set/get-icon'
import { iconToSVG } from '@iconify/utils/lib/svg/build'
import { defaults } from '@iconify/utils/lib/customisations'

console.log(mdi.info) // { height: 24, displayHeight: 24 }
const icon = getIconData(mdi.icons, 'account', true)
console.log(icon) // { width: 16, height: 16 }
const svg = iconToSVG(icon!, defaults)
console.log(svg) // { viewBox: '0 0 16 16' }
cyberalien commented 2 years ago

Oops, sorry. Forgot to add optional properties (type IconifyOptional) to icons.json.

Fixed. New versions of all packages are in process of being published, should take about 10 minutes.

userquin commented 2 years ago

@cyberalien @antfu tested on local with new versions and it is working

cyberalien commented 2 years ago

Published collections list as @iconify/collections. Same as collections.json in @iconify/json, but converted to correct IconifyInfo type.

import { collections } from '@iconify/collections';

console.log(Object.keys(collections));
userquin commented 2 years ago

@cyberalien also tested on node16 with vite + svelte and sveltekit (this 2 guys using target module on their default templates), see the PR comments on unplugin-icons.

antfu commented 2 years ago

unplugin-icons now ships the support for @iconify-json/*: https://twitter.com/antfu7/status/1444125917333262336

Thanks a lot for the awesome works @cyberalien @userquin 🎉