Enhance figma download flow

[yunsii]

Feature request:

Figma images query and download concurrrently

• Use p-limit to implement, allow user to set concurrency limit
• Use axios to replace fetch, axios support more advanced features, like timout, system proxy etc.

Polish query and download print log

• Print loading log with ora
• Print stat log after import task end, specially highlight icons which download error (It's hard to see how many icons download error for now).

Above is what I want to enhance figma download flow at present, I can create a PR for it if the request is allowd. If more features needed, please let me known.

[cyberalien]

Sorry, but I have no plans to implement any of that, it is not a priority.

Concurrency would be nice to have, but currently I have no time to implement that. Amount of work required is not trivial, but benefit is not that clear. If it can speed up fetching by at least 2-3 times, then there would be nice benefit. But I'm not sure there are enough developers using this to warrant all that work.

Adding extra dependencies for logs is a no. More dependencies to manage without any clear benefit is never a good idea.

[yunsii]

Since so, I think I have to implement the features in a independent package. The current functionality can be described as usable, but not user-friendly enough.

[cyberalien]

I agree, it is not user friendly enough.

However, for automated import of icon sets in Iconify, which is its intended purpose, it works. Yes, it is slow, but update is full automated, so for me speed is not a concern.

So I'm not keen on updating it. There is just too much stuff to do right now in other parts of project, which are much higher priority for me.

But PRs are welcome!

[yunsii]

We have to use proxy to achieve better query and download speed in my country, specially for collaborate with others use different proxy approach.

As for print log, it is better to print much less verbose log, also we can use consola to control the print format and level, and support a image download failed callback is a nice try at least.

I can create PRs for all of the enhance figma download flow for now, and it is a good chance to be familiar with the entire project, I can do other higher priority stuff if possible in the future 😄

[cyberalien]

Yes, proxy options do make sense. Did not consider country specific limitations.

Code you need to change to use something else instead of fetch is in src/download/api/download.ts and src/download/api/index.ts

[yunsii]

Well, how about the follow tasks?

• Use axios to replace fetch
• Query and download concurrrently
• Use ora and consola to refactor log printing
• Add a image download failed callback

If yes, I will implement it, you can give me corresponding specific API designs in advance if possible.

[cyberalien]

Really not a fan of changing logging, especially with new dependencies. No idea how it will behave when multiple tasks are ran at the same time, including ones that do not use those logging solutions.

So no to big pull request with those tasks.

However, some parts of it can be implemented and I did test them today:

• Replaced fetch with axios, made it configurable (change axiosConfig before running any tasks)
• Implemented concurrent downloading with retries on failure. Default limit is set to 5 concurrent downloads, maximum of 3 attempts for each download.

Tested that on one Figma repository today, performance difference was massive. Way bigger than I thought.

Implementation is here: https://github.com/iconify/tools/tree/dev/axios. Need to add more unit tests for various scenarios, but it does seem to work correctly so far.

The only added dependency is axios.

[yunsii]

Great, no change of logging is okay, how about provide a download failed callback?

When import from a big figma icons file, it's really hard to know how many icons download failed.

[cyberalien]

Added it, but it is a bit weird. Adding it as parameter would require a lot of refactoring, so instead I went a different route:

• Changed all parameters for concurrent queries to a simple object
• Added optional onfail callback, which allows you to control errors for each query
• Added defaultQueueParams variable, which contains default query parameters: limit (number of concurrent queries), retries (number of attempts), onfail (callback to handle errors)
• onfail query contains index of item that failed, error and original parameters. Original parameters can help identify what exactly failed, but they aren't really useful for Figma.
• In Figma import functions added extra properties for parameters, which can be used in onfail callback to help pinpoint what exactly is failing: it contains name of function that was called: figmaImagesQuery or figmaDownloadImages and payload.

It is a bit messy, but doable.

To handle errors in Figma download functions, you need to add onfail to defaultQueueParams:

defaultQueueParams.onfail = (index, error, params) => {
	// Get Figma functions payload
	const figmaParams = params as unknown as
		| FigmaConcurrentQueriesParams<FigmaConcurrentQueriesParamsFunction>
		| undefined;

	// Contains `function` property, so was imported from Figma
	switch (figmaParams?.function) {
		case 'figmaImagesQuery': {
			// Failed when retrieving URLs for images
			const failedIDs = (
				figmaParams as FigmaConcurrentQueriesParams<'figmaImagesQuery'>
			).payload[index];

			// Got IDs of failed nodes. Do something with it
			throw new Error(
				`Failed to retrieve icons for the following nodes: ${failedIDs.join(
					', '
				)}`
			);
		}

		case 'figmaDownloadImages': {
			// Failed when retriving icon
			const failedItem = (
				figmaParams as FigmaConcurrentQueriesParams<'figmaDownloadImages'>
			).payload[index];

			// Got item. Do something with it
			throw new Error(
				`Failed to retrieve icon for node ${failedItem.id} from ${failedItem.url}`
			);
		}
	}

	// Not a Figma error: re-throw it
	throw error;
};

defaultQueueParams is imported from lib/download/api/queue, Figma* types are imported from lib/import/figma/query

[cyberalien]

Tested in update script with Figma API and custom downloader for Material Symbols. Really fast, no issues.

Merged in next branch.

[yunsii]

Awesome, but I have a question: why is the callback named onfail not onFail?

[cyberalien]

Good question... and fixed.

[yunsii]

I got a new idea: how about to let the import function do not print anything, just expose related callbacks: onFail/onOk or something else, in case it print too many verbose logs.

[cyberalien]

Sure, then you can implement whatever logging functionality you want.

[yunsii]

Using vscode plugin streetsidesoftware.code-spell-checker is a good choice.

[yunsii]

I suggest to export axiosConfig with this:

export const axiosConfig: Omit<
	AxiosRequestConfig,
	'headers' | 'responseType' | 'url' | 'method' | 'data'
> = {
	// Empty by default. Add properties
};

[cyberalien]

Good idea. Removed those properties from axiosConfig.

For logging added 2 methods, allowing maximum flexibility:

1. onStart and onSuccess to defaultQueueParams, similar to onFail above. In these callbacks you have access to queue parameters and Figma stuff.
2. onStart, onSuccess and onError specifically for fetch function, configured in axiosLog in same file as axiosConfig. In these callbacks you have access only to fetch parameters: url, params and error code.

Old basic logging is there, but as a default value for axiosLog.onStart, which you can overwrite.

[yunsii]

Well, I have 2 questions:

• Why not use same callback name onFail or onError, but use both of them? It's a little confused.
• Maybe axiosLog is not a proper name? Though I want to use to logging at present, it is also possible to be used in others cases with someone. And the name seems could not depend on axios, how about apiCallbacks? 😂

[cyberalien]

Fixed. Its onError now and fetchCallbacks since its not specific to axios but to fetching.

[yunsii]

There has one more question: it seems not necessary to expose concurrent callbacks, maybe just fetch callbacks is okay.

They don't care the internal logic is concurrent or not as package users. The import logic should finish all tasks including retries, and invoke user defined fetch callbacks.

In short, No matter how many accidents occur, concurrent logic should finish and callback, that's all.

[cyberalien]

Concurrency is a generic function, which can be used for other tasks, not just fetching, so it is logically separate. Retries are handled by it too.

So logging in concurrency and logging in fetching are 2 completely different unrelated features. In case of Figma API both can be used, depending on what is being logged.

[yunsii]

I will update the package to next version recently, and feedback after trial.

[yunsii]

After upgrade to 4.0.0-beta.7, defaultQueueParams callbacks has no file id, only node id chould be used to print… A bit awkward.

[cyberalien]

In current import process icon names callback is called when icons are imported in icon set, which is happening after all files have been downloaded.

Callback includes full icon code, allowing to change/fix/validate icon to make sure it is correct before importing it in icon set. That would be impossible if names would be assigned before downloading.

[yunsii]

I implement an importFigmaFile function, it will be run concurrently, so if I use defaultQueueParams out of importFigmaFile function, the callbacks has no file id. If use defaultQueueParams in importFigmaFile function, the callbacks will be affected each other I think.

[yunsii]

So I think add callbacks with importFromFigma is more intuitive and friendly.

[yunsii]

If no more changes here, I think use worker pool is a better solution, I'm going to work on it.

[yunsii]

It seems also stucked by vitejs/vite#3932 😂 Because I use vite library mode……

[cyberalien]

Yes, I think this is it for changes. To improve logging further, bigger changes are required.

[cyberalien]

Finally published version 4.0.0, news includes detailed list of changes, docs have been updated.

[yunsii]

After investigation, it still use defaultQueueParams to config importFigmaFile. How to resolve the side effects if I run importFigmaFile concurrently?

[yunsii]

If no more changes, I think I have to implement figma download logic as my expected……