[Ionic 4] Doubt regarding an asynchronous function and how I should correctly run it synchronously

I have the following algorithm:

// Main Function
	// Get some local data
	// Convert to some format I need to edit
	// Loop through user's local images
		// Edit the converted data to add the image's names to it
		// Convert IMAGE to base64 and add result into zip file
		that.uriToBase64(imageFilePath).then((result) => {
			console.log("image as base64: ", result);
			zip.file(arrayItem2.name, result, {base64: true});
		});

	// Convert data again to save it
	// Add data to zip file
	// Generate zip file and save it in user's local storage

//uriToBase64() function

My problem is:

The step ‘Add data to zip file’ happens before the images are added to zip file. Although the ‘Convert IMAGE to base64’ step has a .then, everything inside happens only after everything is finished. In this case, my zip file is being saved without the images. I tried to make this step work synchronously, using async/await syntax in countless ways, but I couldn’t make it work the way I expected, which is adding the image data inside the zip file within each iteration.

Since the result of uriToBase64() function is a Promise, using .then to get the result data should “pause” the loop until the data is added by the zip.file() command, then iterate with the next image, right? If not, what is the correct way to wait for it, taking into account the current structure of the algorithm?


Updates:

Update1

I tried some nasty things to make this work, unsuccessful again.
My new algorithm:

// Main Function
	// Get some local data
	// Convert to some format I need and add to a global variable
	// Loop through user's local images
		// Add +1 to new global variable 'imageCounter'
		// Edit the converted data (now in global variable) to add the image's names to it
		// Convert IMAGE to base64 and add result into zip file
		that.uriToBase64(imageFilePath).then((result) => {
			console.log("image as base64: ", result);
			zip.file(arrayItem2.name, result, {base64: true});
			that.prepareForWrite();

			// Check if this is the last iteration and run function again.
			if (thisIsLastIteration == true) { that.prepareForWrite(); }                        
		});

//prepareForWrite() function
	// imageCounter - 1
	// if imageCounter < 1
		// Convert data again to save it
		// Add data to zip file
		// Generate zip file and save in user's local storage

//uriToBase64() function

This way I receive all the data correctly, but the “Generate zip file and save it in user’s local storage” is adding only the first image, and it ends corrupted.

Add in your function the tag async and await in your method… Like this:

async carregarCarrinho() {

    // Buscando clienteId e empresaId caso eles venham vazios
    if (this.clienteId === undefined || this.empresaId === undefined) {
      await this.clienteEmpresaService.findAllClienteEmpresaByUsuario().subscribe(res => {
        console.log('Pegando resposta da empresa: ', res);
        this.clienteId = res[0].clienteId;
        this.empresaId = res[0].empresaId;
        this.buscarCarrinhoPedido();
      });
    } else {
      this.buscarCarrinhoPedido();
    }
  }

Yes, I already tried some ways with async/await, but without success. Last try was similar to this:

// async Main Function
	// Get some local data
	// Convert to some format I need to edit
	// Loop through user's local images
		// Edit the converted data to add the image's names to it
		// Convert IMAGE to base64 and add result into zip file
		let result = await that.uriToBase64(imageFilePath);
                console.log(result);
                zip.file(arrayItem2.name, result, {base64: true});

	// Convert data again to save it
	// Add data to zip file
	// Generate zip file and save it in user's local storage

//uriToBase64() function

I also divided this main function in parts, and tried to run async/await in the parts that need to go first. No success either.

Imperative and reactive programming are very different mindsets, so attempting to write an Ionic application in an imperative style is going to result in no end of frustration.

Here is a taxonomy I use when writing asynchronous code. I would consider uriToBase64 a “category C” function - we care about what it returns and when, therefore any time we call it, two things about the calling context should be true:

  • from a function whose return type is also a future
  • the first word of which is return

Although those rules may seem arbitrary and capricious, they save us from doomed imperative algorithms. Instead of “loop through user’s local images, doing stuff to each one”, I would suggest thinking in terms of feeding a reactive pipeline:

localFiles(): Promise<string[]> {
  // resolve to a list of things that `imageFilePath` loops across in current code
}

Since uriToBase64 maps one path to a base64 string, we can apply that operation to our pipeline:

localBase64s(): Promise<string[]> {
  return this.localFiles().then(paths => paths.map(path => this.uriToBase64(path)));
}

…and we can continue bubbling that up to create the zip file:

this.localBase64s().then(b64s => {
  // build zip in here and return the completed zip
});
1 Like

Thank you so much for your help, I didn’t even know that this difference existed, I researched a bit to find out more.

Ok, I didn’t quite understand what you initially suggested. At the moment the mentioned loop occurs only to:

  1. acquire the image name;

  2. add the name in an .xml document;

  3. add its base64 in the zip file.

How do I implement this same functionality to the example localFiles() you mentioned? Should I pass the name and uri parameters to this new promise? Should I call this promise within my current loop? Should I create the loop inside the promise?

What I tried:

localFiles(): Promise<string[]> {
	let that = this;
	let filePaths = [];

	// Get every image related to my points
	// Get the 'pointId' parameter from point data
	this.featuresToExport.forEach(function (arrayItem) {
		let pointId = arrayItem.values_.pointId;
	
		// Check if there is any image related to this point
		that.images.forEach(function (arrayItem2) {
			let pointId2 = arrayItem2.pointId;
	
			// If image is related to the point, add reference to XML file
			if (pointId2 == pointId) {

				// Identify to pointId Data node, go up to ExtendedData and append the image next to it.
				$(that.mainDoc).find('Data[name=pointId]').children("value:contains(" + pointId2 + ")").closest("ExtendedData").after('<description>&lt;img src="' + arrayItem2.name + '"&gt;</description>');

				let filePath = arrayItem2.filePath
                                filePaths.push(filePath);
													
			}
		});
	});
	return filePaths;
}

localBase64(): Promise<string[]> {
	return this.localFiles().then(paths => paths.map(path => this.uriToBase64(path)));
}

uriToBase64(uri) {
	console.log('File received to be converted: ', uri);
	let nameFile = uri.replace(this.file.dataDirectory, '');
	console.log("File name to be converted: ", nameFile);
	return this.file.readAsDataURL(this.file.dataDirectory, nameFile).then((file64) => {
		let fileWithoutExtension = ('' + file64 + '').replace(/^data:image\/(png|jpg|jpeg);base64,/, '');
		console.log("File without extension: ", fileWithoutExtension);
		return fileWithoutExtension;
	})
	.catch(err => {
		console.log("Error while transforming image to base64: ", err);
	});		
}

I don’t know if that’s what you meant. Since I’ve never worked with promises this way, I believe I’m returning the localFiles function wrongly, because I get the following error:

Type 'string' is not assignable to type Promise<string[]>

Small Edit
I see now that the promise is suppose to return pathS and not just ONE path. Ok, corrected this.

…which is great, because that error guides you directly to what’s going wrong, which is still a style mismatch. Much of reactive programming consists of thinking like this:

A. what do I need to provide?
B. what is something “one step back” that I could turn into A?
C. how would I turn B into A?

In my previous post, A is “a zipfile of base64 strings”, B was “an array of base64 strings”. Then we repeat the process substituting our array of base64 strings into the A slot, and B became “an array of wanted file paths”. C then was this line:

paths.map(path => this.uriToBase64(path))

…so now “array of wanted file paths” is A, and B would seem to be "an array of potentially wanted image objects and an array of features in featuresToExport". Each “feature” seems to be capable of matching zero or one images, so I would then reframe this as a problem of how to filter this.images to include only those images that match a “feature”. Apologies if I’m not properly understanding what happens when one feature matches multiple images, or one image matches multiple features. I also don’t understand what the jQuery stuff is intending to do, so I’m ignoring it for now.

A side note at this point: if you follow the rule “never type the word function inside the body of one (instead always using arrow functions)”, you also never have to worry about manually doing the ugly “that = this” idiom.

So I think we can actually write localFiles() synchronously if we wanted to:

localFilesSync(): string[] {
  return this.images.filter(pi => {
    // return true if we want pi, false otherwise
    // an image matches a feature if their `pointId` properties compare equal
    return !!(this.featuresToExport.find(pf => pf.values_.pointId == pi.pointId));
    // one more map to extract `filePath` from each desired image
  }).map(image => image.filePath);
}

In English, this more or less says "give me all the elements of images that match something in featuresToExport".

Consumers of localFiles don’t need to know that we can do it synchronously, though, because maybe things will change in the future so that we can’t (need to run things through some asynchronous filter, maybe). Fortunately, it’s trivial to Promise-ify something we already have:

localFiles(): Promise<string[]> {
  return Promise.resolve(this.localFilesSync());
}

So I would suggest letting go of the overarching loop and the thinking behind it. Instead think we’re building a girls’ soccer team. We have a bunch of kids. We send all the boys home (images that don’t match a feature), then we ask each of the girls what their name is (the equivalent of filePath), sew it on the back of a jersey (the base64 transformation), and pack all the jerseys into a box (the zipfile) to travel with the team on the bus to their first game.

1 Like

Thank you for everything. A lot of things to learn in a single post. This is a nice way to think, I can’t change my way of thinking overnight, but I will definitely try.

The Jquery stuff is to edit a XML file, which was not really created in that point. It should be created after getting all image names, so I can insert the names inside the file. I’m doing this inside the loop for now.

I really liked the side note, applying in everything now!

Ok, using the suggested method I was able to get to this part: this.localBase64s().then(b64s => { ... , but I can’t use the b64s. For my understanding this should be an array of base64 strings, but if I console.log(b64s) inside this function the b64s come undefined. Moments later I can see the logs of the uriToBase64() function.

How everything is looking now:

mainFunction() {
	...
        // I believe the function should be called here, correct me if I'm wrong
	this.localBase64s().then(b64s => {
		// build zip in here and return the completed zip
		console.log("b64s: ", b64s);
		return;
	});				
}

// I still using the loop method here because of the Jquery stuff, but I will change later.
localFilesSync(): string[] {
	let that = this;
	let filePaths = [];

	// Get every image related to the points
	this.featuresToExport.forEach((arrayItem) => {
		let pointId = arrayItem.values_.pointId;
	
		// Check if there is any image related to this point
		this.images.forEach((arrayItem2) => {
			let pointId2 = arrayItem2.pointId;
	
			// If image is related to the point, add reference to XML file
			if (pointId2 == pointId) {

				// Identify to pointId Data node, go up to ExtendedData and append the image next to it.
				$(this.mainDoc).find('Data[name=pointId]').children("value:contains(" + pointId2 + ")").closest("ExtendedData").after('<description>&lt;img src="' + arrayItem2.name + '"&gt;</description>');

				let filePath = arrayItem2.filePath;
				filePaths.push(filePath);														
			}
		});
	});

	return filePaths;
}

localFiles(): Promise<string[]> {
	return Promise.resolve(this.localFilesSync());
}

localBase64s(): Promise<void | string[]> {
	return this.localFiles().then(paths => {
		paths.map(path => this.uriToBase64(path));
	});
}

uriToBase64(uri) {
	let nameFile = uri.replace(this.file.dataDirectory, '');
	console.log("File name to be converted: ", nameFile);
	return this.file.readAsDataURL(this.file.dataDirectory, nameFile).then((file64) => {
		let fileWithoutExtension = ('' + file64 + '').replace(/^data:image\/(png|jpg|jpeg);base64,/, '');
		console.log("File without extension: ", fileWithoutExtension);
		return fileWithoutExtension;
	})
	.catch(err => {
		console.log("Error while transforming image to base64: ", err);
	});		
}

There’s an incredibly subtle bug in localBase64s that I suspect you got a hint of with the void that looks out of place in its return value type.

I wrote:

return this.localFiles().then(paths => paths.map(path => this.uriToBase64(path)));

You wrote:

	return this.localFiles().then(paths => {
		paths.map(path => this.uriToBase64(path));
	});

While it is IMHO perfectly reasonable to expect those two to behave the same, life in JavaScriptLand is frequently light-years away from reasonable. My formulation takes advantage of an implicit return value, yours needs an explicit return:

	return this.localFiles().then(paths => {
		return paths.map(path => this.uriToBase64(path));
        ^^^^^^
	});

Absent that, the new array created by paths.map is silently discarded. I suspect your build tools noticed this, and when they told you you needed to add Promise<void> as a possibility for the return type of localBase64s, your radar should have gone off, because that’s a problem, as you’re relying on the Promise returned by localBase64s to resolve to an array of strings. void will not do. This is part of why I keep harping on how important strong typing is.

localBase64s(): Promise<string[]> {
  return this.localFiles().then(paths => paths.map(path => this.uriToBase64(path)));
}

If I write this way I get the following error:

error TS2322: Type 'Promise<string | Promise<string | void>[]>' is not assignable to type 'Promise<string>'.
[ng]       Type 'string | Promise<string | void>[]' is not assignable to type 'string'.
[ng]         Type 'Promise<string | void>[]' is not assignable to type 'string'.

I guess this is because uriToBase64() returns a Promise<string | void>? So what I did earlier to mitigate this error, without knowing that this will discard the array, was add the void tag in localBase64s() and write in that way.

If I add Promise<string> to my uriToBase64() I get a similar error too:

error TS2322: Type 'Promise<string | Promise<string | void>' is not assignable to type 'Promise<string>'.
[ng]       Type 'string | void>' is not assignable to type 'string'.
[ng]         Type 'void' is not assignable to type 'string'.

This is strange, because readAsDataURL() returns a Promise<string>. Do you have any idea why this is happening or how can I fix it?

Exactly, but it shouldn’t.

The catch is the catch, and the easiest way to fix it is to completely take it out:

uriToBase64(uri: string): Promise<string> {
	let nameFile = uri.replace(this.file.dataDirectory, '');
	console.log("File name to be converted: ", nameFile);
	return this.file.readAsDataURL(this.file.dataDirectory, nameFile).then((file64) => {
		let fileWithoutExtension = ('' + file64 + '').replace(/^data:image\/(png|jpg|jpeg);base64,/, '');
		console.log("File without extension: ", fileWithoutExtension);
		return fileWithoutExtension;
	});
}

When you add the catch block, you’re adding an additional code path where the return value of uriToBase64 becomes the result of the catch block (which, as written, is void).

If you want to do more elaborate error handling, one strategy would be to make the catch block return something that can’t be confused with a valid result (such as null), and then filter out the nulls somewhere (most logical place probably right after the array is generated in localBase64s).

1 Like

Nice read and good progress

Consider replacing the forEach-if combo with map(…).filter(…)

Just to be more functional instead of imperative

1 Like

Okay, sorry to disturb again. For now I thought everything was going to be fine, naive me. Even if I remove the catch block, I still get the Type is not assignable error.

error TS2322: Type 'Promise<string[] | Promise<string>[]>' is not assignable to type 'Promise<string[]>'.
[ng]       Type 'string[] | Promise<string>[]' is not assignable to type 'string[]'.
[ng]         Type 'Promise<string>[]' is not assignable to type 'string[]'.
[ng]           Type 'Promise<string>' is not assignable to type 'string'.

I understand that the Promise will be resolved to a string, but shouldn’t the builder know that too? Isn’t that why we explicit put the type tag inside the promise? I tried to change the type of localBase64s() to Promise<any>, which removed this error, but then I can’t access the b64s in this.localBase64s().then(b64s => {.

If I console.log(b64s) here I can see that it return a Promise (ZoneAwarePromise), I can even see the desired value inside __zone_symbol__value,
but I can’t resolve this promise either, since then() does not exist on type any[]. I have reviewed all the inputs and outputs of the functions again and everything seems to be correct. Can you explain me how I correctly match these types?

Array.map returns a new array that consists of a 1-to-1 transformation of the original array. When we said:

paths.map(path => this.uriToBase64(path))

…combined with the return type of uriToBase64, which is a Promise<string>, the result of that expression is going to be an array of Promises: Promise<string>[]. What we actually want is slightly different: a single Promise resolving to an array of strings: Promise<string[]>. Fortunately, there is a standard operator that does exactly that:

Promise.all(paths.map(path => this.uriToBase64(path)))
1 Like

Awesome, I was not expecting this. This part worked like a charm now, sadly I cannot say the same for the whole function.

If I try to run now it will return the same behavior that I was receiving in the beginning: Only the first image is added to the zip file and it is corrupted. The other images and the main data (XML) are ignored.

What I tried to do was loop through each b64 and zip it before move on to zip the main file and generate the zip file. This generate the mentioned error. But then I thought about what we discussed here and rewrited this part. I created a promise just to insert the b64 data into the zip file, then move to the next steps. But sadly the final result was the same.

What I’m doing:

zipBase64s(): Promise<void> {
	return this.localBase64s().then(b64s => {
		b64s.map((b64, index) => {
			this.zip.file(index + ".jpg", b64, {base64:true});
		});
	});
}

And then calling this function in the main function, before everything I mentioned above.

this.zipBase64s().then(() => {
    ...
    // Edited XML back to string
    // Add XML string to zip
    // Generate Zip
}

I believe I’m still doing something wrong, because this zip.file() function is supposed to handle large files and I already have the b64 ready when the function is called. I can’t understand why the corrupted file and the other missing ones, it seems that the function skips it’s job in the middle and go directly to the “generateZip” part.

Would it be possible to provide public access to a repo (such as on GitHub) containing enough code to reproduce the entire problematic feature here (including interfacing with whatever library is doing the zip stuff)?

I created a stackblitz, but I don’t think you be able to use the code there. I couldn’t properly install the JSZip library and the featuresToExport is removed, but I think you won’t need it. Another thing, the uriToBase64() will not work there either, since it expects a local file path, but I inserted two images in assets folder already converted to base64 if you need: image1.txt and image2.txt.

I tried to comment everything I did, but if you have any questions or if the code provided can’t replicate the problem, just ask.


Update 09-05-2015

Okay, I manually loaded two base64 strings into two variables: string64 and string64_2. Then I just zip then by the conventional way:

zip.file("image.jpg", string64, {base64: true});
zip.file("image2.jpg", string64_2, {base64: true});
zip.generateAsync({type:"blob"}).then(...);

If I do this I receive the same corrupted file as mentioned before. So, I can see that there is not wrong with the way we are acquiring the strings, but in the way I’m zipping this files. If I do the same with only one variable, the zip file is created normally without corruption.

So far I was considering that the .file() function was synchronous, because all the examples in the documentation treat it as synchronous, but this behavior makes me believe that it’s not.

With that in mind, I tried this, but I still get the same result.


Another update

Looking a little further in the documentation I found a zip.file('name').async('type') function, which allows you to read a file that has been added and it returns a promise. So I think it is possible to use this function in some way to check if the file is already inserted in the zip file. With that in mind I looked for some related questions, which led me to this question. In this case, each time a file is added, the same function returns the file completion response. What I tried to do:

mainFunction() {
	let string64 = 'veryLongBase64String';

	let b64s = [];
	let arrayOfPromises = [];

	b64s.push(string64);
	b64s.push(string64);
	console.log(b64s);

	b64s.forEach((b64, index) => {
		let fileName = index + ".jpg";
		arrayOfPromises.push(this.addFileToZip(fileName, b64)); 
	});

	Promise.all(arrayOfPromises)
	.then(this.zip.generateAsync({type:"blob"})
		.then((content) => {
			let filePath = this.file.externalRootDirectory + '/sfmapp_downloads/';
			this.file.writeFile(filePath, "testZip.zip", content).then(() => {
				alert("File saved!");
			})
			.catch((err) => {
				alert("Error while creating file: " + JSON.stringify(err));
			});
		})
	);
}

addFileToZip(name: string, b64: string): Promise<string> {
	this.zip.file(name, b64, {base64: true});
	return this.zip.file(name).async("base64");
}

Unfortunately I still get the corrupted file.

Well, I’m not sure how helpful this is, but since I wasn’t able to get your stackblitz into a format suitable for playing around with, I made a scratch app that does seem to work, in that the downloaded zip files seem to be uncorrupted. Haven’t tested it extensively, but see if you can adapt it to your situation. Now that I know what zip library you’re using, we can take advantage of the fact that it accepts Promises in the file method, making things a bit simpler.

home.page.html

<ion-content>
    <ion-list>
        <ion-item *ngFor="let fc of fcs">
            <ion-input [formControl]="fc"></ion-input>
        </ion-item>
    </ion-list>

    <ion-button (click)="addRaw()">add</ion-button>
    <ion-button (click)="doZip()">zip</ion-button>
    <ion-button (click)="downloadZip()">download</ion-button>
</ion-content>

home.page.ts

export class HomePage {
    fcs: FormControl[] = [];
    zipped: ArrayBuffer | null = null;

    constructor() {
    }

    addRaw(): void {
        this.fcs.push(new FormControl());
    }

    b64ify(raw: string): Promise<string> {
        return Promise.resolve(btoa(raw));
    }

    b64ifyAll(): Promise<string>[] {
        return this.fcs.map(fc => this.b64ify(fc.value));
    }

    doZip(): void {
        let b64ps = this.b64ifyAll();
        let zipper = new JSZip();
        for (let i = 0; i < b64ps.length; ++i) {
            zipper = zipper.file(`${i}.txt`, b64ps[i], {base64: true});
        }
        zipper.generateAsync({type: "arraybuffer"}).then(zar => {
            this.zipped = zar;
        });
    }

    downloadZip(): void {
        let filename = "scratch.zip";
        let blob = new Blob([this.zipped!], {type: "application/zip"});
        let url = window.URL.createObjectURL(blob);
        let a = document.createElement('a');
        a.setAttribute('href', url);
        a.setAttribute('download', filename);
        a.style.display = 'none';
        document.body.appendChild(a);
        a.click();
        document.body.removeChild(a);
    }
}

It’s pretty bare-bones, but I think incorporates all of the core aspects of your problem. Click “add” as many times as needed to provide new text input areas to type stuff into, type stuff into them, click “zip” to do the zipping, and then “download” to download the resultant zip file. Promise.resolve(b2a(raw)) uses the internal JavaScript base64er and Promise-ifies it to make sure it would work with any other asynchronous source. We don’t need Promise.all any more, because we’re feeding JSZip each Promise instead of resolving them in app code.

2 Likes

Okay, looking at your code, I see that it’s simpler than anything we’ve been through so far, which I found odd. I saw that you used “arraybuffer” as the file type generated by generateAsync(), if I change this in my code, which was so far blob, everything works as expected. All images and the main file are added and the file is generated without errors.

I thank you so much for all your help and teachings made available in these posts, I hope other people can be helped from them too.

One last question to conclude: Why did you use this type of format?

1 Like

And I thank you for your diligent following up and effort incorporating the thoughts we developed here.

Ironically only because I cribbed downloadZip from my toolbox of existing code, and since it already makes its own Blobs, I just needed anything that the Blob constructor would be happy with. The app I happened to take it from happened to be using an ArrayBuffer. Now I wonder what other formats may have worked for you.

2 Likes