Skip to content

Refactor/2116 2 #2474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
justinmmott wants to merge 2 commits into
base: ExperimentalTabMode
Choose a base branch
from
Draft

Refactor/2116 2 #2474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9508c45
replaced .then()s with await in all but the test files
justinmmott Feb 13, 2026
b8a48dc
replaced .then()s with await in the test files
justinmmott Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 90 additions & 120 deletions eslint/pack.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@ var extractFileListFromHtml = function(htmlAsString) {
return [];
};

var getFileList = function(fileName) {
return readFilePromise(fileName).then(function(data) {
return extractFileListFromHtml(data.toString());
});
var getFileList = async function(fileName) {
let data = await readFilePromise(fileName);
return extractFileListFromHtml(data.toString());
};

var adjustedFileListForEslint = function(fileList) {
Expand Down Expand Up @@ -55,18 +54,15 @@ var writeFilePromise = function(fileName, buffer) {
};

// package geting all the files
var readAllFiles = function(fileList, loadedFiles) {
return fileList.reduce(function(sequence, fileName) {
return sequence.then(function() {
return readFilePromise(fileName);
}).then(function(data) {
console.log("saving file: " + fileName);
loadedFiles.push({
fileName: fileName,
text: data.toString()
});
var readAllFiles = async function(fileList, loadedFiles) {
return await fileList.forEach(async function(fileName) {
Copy link

@zolantris zolantris Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reduce should be used here not a forEach. Return of forEach will be undefined. (if this is an array)

See this Doc on Array.prototype.forEach. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/forEach#return_value

What could be optimized is flattening the functions. Use an await a Promise.All for each of these items,

And each promise item in an array.

After promise.all you can return the array and it will be for all fulfilled promises. But it would throw a error still if anything failed in promise.all

Copy link

@zolantris zolantris Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regardless.

Would be good to have unit tests of this functionality to prove it's not thread blocking at the await syntax similar to original implementation and that the return is as expected

let data = await readFilePromise(fileName);
console.log("saving file: " + fileName);
loadedFiles.push({
fileName: fileName,
text: data.toString()
});
}, Promise.resolve());
});
};

var countLines = function(fileText) {
Expand All @@ -77,56 +73,25 @@ var makeIndexLine = function(fileName, startIndex, count) {
return "\"" + fileName + "\", " + (startIndex + 1) + ", " + (startIndex + count) + "\r\n";
};


//=================================================================
// pack source into packed.js So its easy to be examined with eslint
// just run eslint against packed.js

var loadedFiles = [];
getFileList("../plugin/popup.html").then(function(fileList) {
fileList = adjustedFileListForEslint(fileList);
console.log(fileList);
return readAllFiles(fileList, loadedFiles);
}).then(function() {
let temp = "";
let lineCount = 0;
let index = "";
for (let f of loadedFiles) {
temp += f.text;
let count = countLines(f.text);
index += makeIndexLine(f.fileName, lineCount, count);
lineCount += count;
}
fs.writeFileSync("packed.js", temp);
fs.writeFileSync("index.csv", index);
}).catch(function(err) {
console.log(err);
});

//=================================================================
// This is bit where we pack the extension into a zip & xpi files.

var addToZipFile = function(zip, nameInZip, filePath) {
return readFilePromise(filePath).then(function(data) {
zip.add(nameInZip, new zipjs.Uint8ArrayReader(data));
});
var addToZipFile = async function(zip, nameInZip, filePath) {
let data = await readFilePromise(filePath);
zip.add(nameInZip, new zipjs.Uint8ArrayReader(data));
};

var writeZipToDisk = function(zip, filePath) {
var writeZipToDisk = async function(zip, filePath) {
console.log("writeZipToDisk " + filePath);
return zip.close().then(function(buffer) {
buffer.arrayBuffer().then(function(arraybuffer) {
return writeFilePromise(filePath, arraybuffer);
});
});
let buffer = await zip.close();
let arraybuffer = await buffer.arrayBuffer();
await writeFilePromise(filePath, arraybuffer);
};

var addFilesToZip = function(zip, fileList) {
return fileList.reduce(function(sequence, fileName) {
return sequence.then(function() {
return addToZipFile(zip, fileName, "../plugin/" + fileName);
});
}, Promise.resolve());
var addFilesToZip = async function(zip, fileList) {
return await fileList.forEach(async function(fileName) {
Copy link

@zolantris zolantris Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same undefined bug as above.

Copy link

@zolantris zolantris Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tests for inputs/outputs please!

Copy link
Owner

@dteviot dteviot Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zolantris

eslint/pack.js is a build script. Providing tests is somewhat redundant as it's purpose is to assemble the files into the WebToEpub extension. So, it should either work or not. Easiest way to check is run script before changes and save WebToEpub, then make changes, build again and compare built WebToEpub. Should be same.

return await addToZipFile(zip, fileName, "../plugin/" + fileName);
});
};

var getLocaleFilesNames = function() {
Expand All @@ -141,22 +106,18 @@ var getLocaleFilesNames = function() {
});
};

var addPopupHtmlToZip = function(zip) {
return readFilePromise("../plugin/popup.html")
.then(function(data) {
let htmlAsString = data.toString()
.split("\r")
.filter(s => !s.includes("/experimental/"))
.join("\r");
zip.add("popup.html", new zipjs.TextReader(htmlAsString));
});
var addPopupHtmlToZip = async function(zip) {
let data = await readFilePromise("../plugin/popup.html");
let htmlAsString = data.toString()
.split("\r")
.filter(s => !s.includes("/experimental/"))
.join("\r");
zip.add("popup.html", new zipjs.TextReader(htmlAsString));
};

var addBinaryFileToZip = function(zip, fileName, nameInZip) {
return readFilePromise(fileName)
.then(function(data) {
zip.add(nameInZip, new zipjs.Uint8ArrayReader(data));
});
var addBinaryFileToZip = async function(zip, fileName, nameInZip) {
let data = await readFilePromise(fileName);
zip.add(nameInZip, new zipjs.Uint8ArrayReader(data));
};

var addImageFileToZip = function(zip, fileName) {
Expand All @@ -169,44 +130,29 @@ var addCssFileToZip = function(zip, fileName) {
return addBinaryFileToZip(zip, "../plugin/" + dest, dest);
};

var packNonManifestExtensionFiles = function(zip, packedFileName) {
return addBinaryFileToZip(zip, "../plugin/book128.png", "book128.png")
.then(function() {
return addImageFileToZip(zip, "ChapterStateDownloading.svg");
}).then(function() {
return addImageFileToZip(zip, "ChapterStateLoaded.svg");
}).then(function() {
return addImageFileToZip(zip, "ChapterStateNone.svg");
}).then(function() {
return addImageFileToZip(zip, "ChapterStateSleeping.svg");
}).then(function() {
return addImageFileToZip(zip, "FileEarmarkCheck.svg");
}).then(function() {
return addImageFileToZip(zip, "FileEarmarkCheckFill.svg");
}).then(function() {
return addCssFileToZip(zip, "default.css");
}).then(function() {
return addCssFileToZip(zip, "alwaysDark.css");
}).then(function() {
return addCssFileToZip(zip, "autoDark.css");
}).then(function() {
return getFileList("../plugin/popup.html");
}).then(function(fileList) {
return getLocaleFilesNames().then(function(localeNames) {
return ["js/ContentScript.js"].concat(localeNames)
.concat(fileList.filter(n => !n.includes("/experimental/")));
});
}).then(function(fileList) {
return addFilesToZip(zip, fileList);
}).then(function() {
return addPopupHtmlToZip(zip);
}).then(function() {
return writeZipToDisk(zip, packedFileName);
}).then(function() {
console.log("Wrote Zip to disk");
}).catch(function(err) {
console.log(err);
});
var packNonManifestExtensionFiles = async function(zip, packedFileName) {
try {
await addBinaryFileToZip(zip, "../plugin/book128.png", "book128.png");
await addImageFileToZip(zip, "ChapterStateDownloading.svg");
await addImageFileToZip(zip, "ChapterStateLoaded.svg");
await addImageFileToZip(zip, "ChapterStateNone.svg");
await addImageFileToZip(zip, "ChapterStateSleeping.svg");
await addImageFileToZip(zip, "FileEarmarkCheck.svg");
await addImageFileToZip(zip, "FileEarmarkCheckFill.svg");
await addCssFileToZip(zip, "default.css");
await addCssFileToZip(zip, "alwaysDark.css");
await addCssFileToZip(zip, "autoDark.css");
let fileList = await getFileList("../plugin/popup.html");
let localeNames = await getLocaleFilesNames();
["js/ContentScript.js"].concat(localeNames).concat(fileList.filter(n => !n.includes("/experimental/")));
Copy link
Owner

@dteviot dteviot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be

fileList = ["js/ContentScript.js"].concat(localeNames).concat(fileList.filter(n => !n.includes("/experimental/")));

So that next line processes the correct list of files

await addFilesToZip(zip, fileList);
await addPopupHtmlToZip(zip);
await writeZipToDisk(zip, packedFileName);
console.log("Wrote Zip to disk");
}
catch (err) {
console.log(err);
}
};

var makeManifestForFirefox = function(data) {
Expand Down Expand Up @@ -239,18 +185,42 @@ var makeManifestForChrome = function(data) {
return manifest;
};

var packExtension = function(manifest, fileExtension) {
var packExtension = async function(manifest, fileExtension) {
let zipFileWriter = new zipjs.BlobWriter("application/epub+zip");
let zipWriter = new zipjs.ZipWriter(zipFileWriter, {useWebWorkers: false,compressionMethod: 8, extendedTimestamp: false});
zipWriter.add("manifest.json", new zipjs.TextReader(JSON.stringify(manifest)));
return packNonManifestExtensionFiles(zipWriter, "WebToEpub" + manifest.version + fileExtension);
};

// pack the extensions for Chrome and firefox
readFilePromise("../plugin/manifest.json")
.then(function(data) {
packExtension(makeManifestForFirefox(data), ".xpi");
packExtension(makeManifestForChrome(data), ".zip");
}).catch(function(err) {
return await packNonManifestExtensionFiles(zipWriter, "WebToEpub" + manifest.version + fileExtension);
};


(async () => {
Copy link

@zolantris zolantris Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a function like init() that can be called. Call it afterwards but wrap it in a process.env variable so init is not called on import/require

This would make the code much more testable by not forcing execution on evaluation.

//=================================================================
// pack source into packed.js So its easy to be examined with eslint
// just run eslint against packed.js
try {
var loadedFiles = [];
let fileList = await getFileList("../plugin/popup.html");
fileList = adjustedFileListForEslint(fileList);
console.log(fileList);

await readAllFiles(fileList, loadedFiles);
let temp = "";
let lineCount = 0;
let index = "";
for (let f of loadedFiles) {
temp += f.text;
let count = countLines(f.text);
index += makeIndexLine(f.fileName, lineCount, count);
lineCount += count;
}
fs.writeFileSync("packed.js", temp);
fs.writeFileSync("index.csv", index);

// pack the extensions for Chrome and firefox
let data = await readFilePromise("../plugin/manifest.json");
await packExtension(makeManifestForFirefox(data), ".xpi");
await packExtension(makeManifestForChrome(data), ".zip");
} catch (err) {
console.log(err);
});
}
})();
17 changes: 9 additions & 8 deletions plugin/js/Download.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,19 @@ class Download {
}
}

static saveOnFirefox(options, cleanup) {
return browser.runtime.getPlatformInfo().then(platformInfo => {
static async saveOnFirefox(options, cleanup) {
try {
let platformInfo = await browser.runtime.getPlatformInfo();
if (Download.isAndroid(platformInfo)) {
Download.saveOnFirefoxForAndroid(options, cleanup);
} else {
return browser.downloads.download(options).then(
// on Firefox, resolves when "Save As" dialog CLOSES, so no
// need to delay past this point.
downloadId => Download.onDownloadStarted(downloadId, cleanup)
);
// on Firefox, resolves when "Save As" dialog CLOSES, so no
// need to delay past this point.v
Download.onDownloadStarted(await browser.downloads.download(options), cleanup);
}
}).catch(cleanup);
} catch {
cleanup();
}
}

static saveOnFirefoxForAndroid(options, cleanup) {
Expand Down
55 changes: 24 additions & 31 deletions plugin/js/HttpClient.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,42 +297,35 @@ class FetchResponseHandler {
this.contentType = response.headers.get("content-type");
}

extractContentFromResponse(response) {
async extractContentFromResponse(response) {
if (this.isHtml()) {
return this.responseToHtml(response);
return await this.responseToHtml(response);
} else {
return this.responseToBinary(response);
return await this.responseToBinary(response);
}
}

responseToHtml(response) {
return response.arrayBuffer().then(function(rawBytes) {
let data = this.makeTextDecoder(response).decode(rawBytes);
let html = new DOMParser().parseFromString(data, "text/html");
util.setBaseTag(this.response.url, html);
this.responseXML = html;
return this;
}.bind(this));
async responseToHtml(response) {
let rawBytes = await response.arrayBuffer();
let data = this.makeTextDecoder(response).decode(rawBytes);
let html = new DOMParser().parseFromString(data, "text/html");
util.setBaseTag(this.response.url, html);
this.responseXML = html;
}

responseToBinary(response) {
return response.arrayBuffer().then(function(data) {
this.arrayBuffer = data;
return this;
}.bind(this));
async responseToBinary(response) {
let data = await response.arrayBuffer();
this.arrayBuffer = data;
}

responseToText(response) {
return response.arrayBuffer().then(function(rawBytes) {
return this.makeTextDecoder(response).decode(rawBytes);
}.bind(this));
async responseToText(response) {
let rawBytes = await response.arrayBuffer();
this.makeTextDecoder(response).decode(rawBytes);
}

responseToJson(response) {
return response.text().then(function(data) {
this.json = JSON.parse(data);
return this;
}.bind(this));
async responseToJson(response) {
let data = await response.text();
this.json = JSON.parse(data);
Copy link
Owner

@dteviot dteviot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not returning this

}

makeTextDecoder(response) {
Expand All @@ -358,8 +351,8 @@ class FetchJsonResponseHandler extends FetchResponseHandler {
super();
}

extractContentFromResponse(response) {
return super.responseToJson(response);
async extractContentFromResponse(response) {
return await super.responseToJson(response);
}
}

Expand All @@ -368,8 +361,8 @@ class FetchTextResponseHandler extends FetchResponseHandler {
super();
}

extractContentFromResponse(response) {
return super.responseToText(response);
async extractContentFromResponse(response) {
return await super.responseToText(response);
}
}

Expand All @@ -378,7 +371,7 @@ class FetchHtmlResponseHandler extends FetchResponseHandler {
super();
}

extractContentFromResponse(response) {
return super.responseToHtml(response);
async extractContentFromResponse(response) {
return await super.responseToHtml(response);
}
}
Loading