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

feat: esm support icon set + pnpm workspaces #18

Closed userquin closed 2 years ago

userquin commented 2 years ago

I'll try to resolve conflicts

userquin commented 2 years ago

I have merge from next, and then change some prettier changes you have included.

I have left the prettier files, change them again if you want.

userquin commented 2 years ago

If merged, we'll need to update each readme inside packages and the root readme

userquin commented 2 years ago

And sorry I should have do a rebase instead merging and then undoing changes...

cyberalien commented 2 years ago

I wanted to merge changes to quickly resolve all conflicts, so this branch could be easily merged into next.

However I've never used Yarn workspaces before, so now there is an issue I can't solve: can't run build script. It complains that tsup isn't installed. No worries, added tsup to main package.json. Now it complains about next package. So my question is, do all packages used by workspaces need to be listed in main package.json?

userquin commented 2 years ago

@cyberalien do you mean pnpm workspaces? If you're not confortable with pnpm just switch to yarn.

The dependencies are shared using the lock file, you should install on each workspace/package as usual but using pnpm i [-D] xxx from inside the workspace/package. To add a dep to root just use pnpm i [-D] zzz -w (-w will do the trick, if not provided pnpm will complain).

I think you need to remove shamefully-hoist=false entry from <root>/.npmrc file.

I usually add each workspace/package build to the root "build-xxx": "npm -C packages/xxx run build" and so I manage it from root folder but you can also go to packages/xxx directory and run pnpm run build.

To add a new workspace/package you can just create the directory and add it to pnpm-workspace.yaml: to test your json files for @iconify-json/* I suggest you to add this to pnpm-workspace.yaml:

packages:
  - '@iconify/*'
  - examples/*
  - '@iconify-json/*'

then create @iconify-json directory and add inside each workspace/package: for example mdi, fa and so on...

This way the name is the name used once published and the same we install them on target projects.

You can then add another workspace/package to test your @iconify-json/*, then include on its package.json the required dependencies using the workspace: protocol (take a look at any examples workspace/directory):

"devDependencies": {
  "@iconify/icon-set-resolver": "workspace:*",
  "@iconify-json/mdi": "workspace:*",
   <other-dev-dependencies>
}

Then, add the build-mdi to root package.json scripts and the new tester workspace/package and from root invoke the build-mdi and the test. This script maybe can be build-json-collection pointing to the collection you want to test, just modify the workspace/package you want to test:

"scripts": {
  "build-json-collection": "npm -C @iconify-json/mdi run build"
}

Since you will not publish the @iconify-json/* package from here, you can exclude @iconify-json/* from git.

To update dependencies, you should install taze from here (add it to root package.json dev dependencies): https://github.com/antfu/taze, this way you update all versions of all workspace/packages only with one command, instead doing manually:

You can also take a look a antfu/ni here https://github.com/antfu/ni.

cyberalien commented 2 years ago

Thanks! I got confused by yarn.lock and didn't read pull request properly. Thank you for detailed explanation.

userquin commented 2 years ago

Thanks! I got confused by yarn.lock and didn't read pull request properly. Thank you for detailed explanation.

about taze and ni packages, install them globally and so you can use taze and ni variant commands where you want.

userquin commented 2 years ago

@cyberalien we can also avoid using dynamic import via loading directly the json file or usingcreateRequire approach, seems to be not working the dynamic import on some environments.

Instead using this, from my branch here https://github.com/userquin/collections-json/blob/feat/esm-support-icon-set/%40iconify/icon-set-resolver/src/index.ts:

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

we can use the unplugin-icons approach, loading json file, from here https://github.com/antfu/unplugin-icons/blob/main/src/core/modern.ts#L38 (using local-pkg), or just using createRequire approach from here https://github.com/antfu/unplugin-icons/pull/71#issuecomment-927814752

userquin commented 2 years ago

@cyberalien you can take a look at this repo, using unocss with preset-icons: https://github.com/userquin/unocss-prefer-bgimage-icons.

On vite.config.ts file you can play with icons of any collection: just add @iconify-json/<collection> as dev dependency, then go to vite.config.ts file and filter icons, changing collection and adding some filter on transform vite hook, right now showing only first 1000 icons of mdi, but you can apply what you want.

If you include the repo on a workspace here, using pnpm you can test all your icons just changing dev dependencies on the workspace instead installing it from npm registry.

I include the version of svg icons using backgound image, just play with the check on the app.

EDIT: you will need node 14 and pnpm to run the repo linked

userquin commented 2 years ago

You can see it here https://imgur.com/onEoV63

Also running on android device to test masked icons (the vite app dist served by the apk, no server interaction): https://joaqun70556742.imgur.com/all/?third_party=1 and here on android emulator https://joaqun70556742.imgur.com/all/?third_party=1

cyberalien commented 2 years ago

Very impressive!!!

Looks like I'll be playing with UnoCSS next few days. 😍

userquin commented 2 years ago

Very impressive!!!

Looks like I'll be playing with UnoCSS next few days. 😍

take a look at this 2 entries from Anthony's blogs:

userquin commented 2 years ago

upps, the links for android emulator are wrong:

cyberalien commented 2 years ago

Thanks!

On topic of workspaces, any advantages of using PNPM instead of NPM? As of version 7 NPM supports workspaces too, I've been using it in new version Iconify Tools. NPM 7 comes with Node 16, which as of few weeks ago is the latest recommended version.

userquin commented 2 years ago

No problem using any package manager workspace solution: lerna, pnpm workspaces or npm workspaces.

The repo is to allow you testing new collections or new icons on existing collections, on the repo you want, so feel free to use it. You can also use the full version of iconify, just change the virtual module to load what you want.

EDIT: since the icons are loaded from disk using local-pkg you can just drop the json file(s) and use it as a seperate repo you can have on your local (you only need to read from disk).

cyberalien commented 2 years ago

I'm not sure about merging this because this repo is also used for other languages that do not support monorepos and/or do not need Node.js stuff. It is essentially a collection of JSON files, which are language independent.

Maybe move proposed icon-set-resolver functions to utils package?

userquin commented 2 years ago

close it