Skip to content

Commit da779b5

Browse files
authored
Avoid emitting extra blank lines for JS library items in jsifier.mjs (emscripten-core#28044)
This change is mostly about cleaning up the code in `addFromLibrary`. Previously, `depsText` in `src/jsifier.mjs` had two issues that caused extra blank lines in unoptimized JS output: 1. When `addFromLibrary` was called on an already-emitted dependency, it returned `undefined`. Using `.filter((x) => x != '')` did not filter out `undefined`, causing `.join('\n')` to emit an empty line for each already-emitted dependency. 2. Because `deps` is always an array (`??= []`), the `deps ? ... : ''` ternary always took the truthy branch and appended `'\n'`, even when no dependencies were emitted. This gave every `addFromLibrary` return value a leading `'\n'`. Use a `joinChunks` helper (`chunks.filter(Boolean).join('\n')`) to join non-empty dependency, doc comment, and content strings without extra newlines. Also update `indentify` in `src/parseTools.mjs` to indent the first line of non-empty strings, which previously only happened because of the accidental leading `'\n'` on each item.
1 parent 45a6cff commit da779b5

4 files changed

Lines changed: 18 additions & 33 deletions

File tree

‎src/jsifier.mjs‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -868,10 +868,7 @@ function(${args}) {
868868
// Add the docs if they exist and if we are actually emitting a declaration.
869869
// See the TODO about wasmTable above.
870870
let docs = LibraryManager.library[symbol + '__docs'];
871-
let commentText = '';
872-
if (contentText != '' && docs) {
873-
commentText += docs + '\n';
874-
}
871+
const commentText = contentText && docs ? docs : '';
875872

876873
if (EMIT_TSD) {
877874
LibraryManager.libraryDefinitions[mangled] = {
@@ -880,22 +877,20 @@ function(${args}) {
880877
};
881878
}
882879

883-
const depsText = deps
884-
? deps
885-
.map(addDependency)
886-
.filter((x) => x != '')
887-
.join('\n') + '\n'
888-
: '';
880+
const joinChunks = (chunks) => chunks.filter(Boolean).join('\n');
881+
882+
const depsText = joinChunks(deps.map(addDependency));
883+
889884
// Defer direct JS value aliases (`var _alias = _target;`) to `libraryAliases`
890885
// so the target symbol is guaranteed to be defined first. Skip native
891886
// aliases (which emit no code here) and signature-mismatch aliases (where
892887
// `isFunction` is set because they are emitted as forwarding wrapper
893888
// functions rather than immediate value assignments).
894889
if (aliasTarget && !isNativeAlias && !isFunction) {
895-
libraryAliases.push(commentText + contentText);
896-
return depsText.trim();
890+
libraryAliases.push(joinChunks([commentText, contentText]));
891+
return depsText;
897892
}
898-
return depsText + commentText + contentText;
893+
return joinChunks([depsText, commentText, contentText]);
899894
}
900895

901896
const JS = addFromLibrary(symbol, 'root reference (e.g. compiled C/C++ code)');

‎src/parseTools.mjs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,13 +346,13 @@ function splitI64(value) {
346346

347347
export function indentify(text, indent) {
348348
// Don't try to indentify huge strings - we may run out of memory
349-
if (text.length > 1024 * 1024) return text;
349+
if (!text || text.length > 1024 * 1024) return text;
350350

351351
indent = ' '.repeat(indent);
352352

353353
// Perform indentation in a smart fashion that does not leak indentation
354354
// inside multiline strings enclosed in `` characters.
355-
let out = '';
355+
let out = indent;
356356
for (let i = 0; i < text.length; ++i) {
357357
// Output a C++ comment as-is, don't get confused by ` inside a C++ comment.
358358
if (text[i] == '/' && text[i + 1] == '/') {

‎test/codesize/test_codesize_minimal_O0.expected.js‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -712,42 +712,33 @@ async function createWasm() {
712712

713713
// Begin JS library code
714714

715-
716715
class ExitStatus {
717716
name = 'ExitStatus';
718717
constructor(status) {
719718
this.message = `Program terminated with exit(${status})`;
720719
this.status = status;
721720
}
722721
}
723-
724722
/** @type {!Int32Array} */
725723
var HEAP32;
726-
727724
/** @type {!Int8Array} */
728725
var HEAP8;
729-
730726
/** @type {!Uint32Array} */
731727
var HEAPU32;
732-
733728
var callRuntimeCallbacks = (callbacks) => {
734729
while (callbacks.length > 0) {
735730
// Pass the module as the first argument.
736731
callbacks.shift()(Module);
737732
}
738733
};
739-
740734
function ptrToString(ptr) {
741735
assert(typeof ptr === 'number', `ptrToString expects a number, got ${typeof ptr}`);
742736
// Convert to 32-bit unsigned value
743737
ptr >>>= 0;
744738
return '0x' + ptr.toString(16).padStart(8, '0');
745739
}
746-
747740
var stackRestore = (val) => __emscripten_stack_restore(val);
748-
749741
var stackSave = () => _emscripten_stack_get_current();
750-
751742
var warnOnce = (text) => {
752743
warnOnce.shown ||= {};
753744
if (!warnOnce.shown[text]) {
@@ -757,7 +748,6 @@ async function createWasm() {
757748
}
758749
};
759750

760-
761751
// End JS library code
762752

763753
// include: postlibrary.js
Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
{
2-
"hello_world.js": 53049,
3-
"hello_world.js.gz": 16879,
2+
"hello_world.js": 53002,
3+
"hello_world.js.gz": 16858,
44
"hello_world.wasm": 14279,
55
"hello_world.wasm.gz": 7101,
6-
"no_asserts.js": 22107,
7-
"no_asserts.js.gz": 7869,
6+
"no_asserts.js": 22074,
7+
"no_asserts.js.gz": 7852,
88
"no_asserts.wasm": 11393,
99
"no_asserts.wasm.gz": 5646,
10-
"strict.js": 50177,
11-
"strict.js.gz": 15887,
10+
"strict.js": 50132,
11+
"strict.js.gz": 15873,
1212
"strict.wasm": 14279,
1313
"strict.wasm.gz": 7098,
14-
"total": 165284,
15-
"total_gz": 60480
14+
"total": 165159,
15+
"total_gz": 60428
1616
}

0 commit comments

Comments
 (0)