Move from JSDom to linkedom

[PaulAdamDavis]

No description provided.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/mg-wp-api/utils/users-html-to-json.js (1)

19-19: ⚠️ Potential issue | 🟡 Minor

Typo in variable name: desitnationDir should be destinationDir.

📝 Proposed fix

-const desitnationDir = dirname(process.argv[2]);
+const destinationDir = dirname(process.argv[2]);
-const destPath = join(desitnationDir, 'users.json');
+const destPath = join(destinationDir, 'users.json');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mg-wp-api/utils/users-html-to-json.js` at line 19, Rename the
misspelled variable desitnationDir to destinationDir where it's declared and in
all usages (the declaration const desitnationDir = dirname(process.argv[2]); and
any references that follow) so the identifier is correct and consistent; update
any imports/exports or references within the same file that rely on
desitnationDir to use destinationDir instead.

🧹 Nitpick comments (3)

packages/mg-ghost-api/lib/processor.js (1)

83-83: Unused second argument in processPosts call.

processPosts is called with (input.posts, output.users) but the function signature on line 49 only accepts posts. The second argument is silently ignored—consider removing it for clarity.

Proposed fix

-    output.posts = processPosts(input.posts, output.users);
+    output.posts = processPosts(input.posts);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mg-ghost-api/lib/processor.js` at line 83, Call to processPosts
passes an unused second argument (output.users) which the function processPosts
(declared around line 49) does not accept; remove the extraneous argument by
changing the call that sets output.posts from processPosts(input.posts,
output.users) to just processPosts(input.posts), or if user data is actually
required update the processPosts function signature to accept and use users,
referencing processPosts and the output.posts assignment so the change is
applied consistently.

packages/mg-wp-api/test/process.test.js (1)

511-532: These assertions are now too serializer-specific.

They lock in linkedom's current attribute ordering and CSS formatting rather than the behavior being tested, so another harmless serializer change will churn these tests again. Narrower assert.match(...) checks — or reparsing the output and asserting on audio[style="width:100%"], audio[autoplay], audio[loop], and the Twitter script[src][async] shape — would be more stable.

Also applies to: 640-640

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mg-wp-api/test/process.test.js` around lines 511 - 532, The tests
assert.equal on the entire serialized HTML (using processor.processContent) are
too brittle; change them to verify behavior only by either using assert.match
with a regex for presence of the audio element and attributes (e.g.
style="width:100%", presence of autoplay and loop attributes) or parse the
processed HTML with linkedom/DOM parsing and assert that the audio element has
style width:100%, has attributes autoplay and/or loop as expected and that the
twitter script element has src and async attributes; replace references to
assert.equal in these test cases with assertions that check
audio[style="width:100%"], audio[autoplay], audio[loop], and script[src][async]
rather than exact serializer output (affecting the tests that call
processor.processContent and currently use assert.equal).

packages/mg-wp-api/lib/processor.js (1)

1327-1332: Sequential processing replaces batched parallel execution.

The previous implementation used BATCH_SIZE = 100 with Promise.all for parallel processing. This sequential approach with null-out for memory management may reduce throughput for large migrations, but provides better memory characteristics.

Consider whether this trade-off aligns with operational requirements for large WP API imports.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/mg-wp-api/lib/processor.js` around lines 1327 - 1332, The current
for-loop processes posts sequentially calling processPost and nulling entries,
which removes the prior batched parallelism (BATCH_SIZE) and reduces throughput;
change this to process posts in chunks using the existing BATCH_SIZE constant
(or define one) and run each chunk through Promise.all to execute
processPost(posts[i], users, options, errors, fileCache) in parallel, push all
resolved results into results, and null out processed posts to preserve memory
characteristics—keep using the same variables (posts, results, processPost,
users, options, errors, fileCache) and ensure errors from Promise.all are
handled/propagated similarly to the previous implementation.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/mg-medium-export/lib/process.js`:
- Around line 14-20: The loop currently mutates input.posts by setting
input.posts[i] = null, turning a pure transform into a destructive API; remove
that mutation so the function remains non-destructive: in the loop inside the
code that builds output.posts, read each entry into a local variable (e.g.,
const post = input.posts[i]), skip null/undefined entries, call processPost({
name: post.name, html: post.html, globalUser, options }) and push the result to
output.posts, and do not assign null back to input.posts[i]; if consuming
behavior is desired instead, make it explicit via an option or a separate
function name rather than silently mutating input.posts.

---

Outside diff comments:
In `@packages/mg-wp-api/utils/users-html-to-json.js`:
- Line 19: Rename the misspelled variable desitnationDir to destinationDir where
it's declared and in all usages (the declaration const desitnationDir =
dirname(process.argv[2]); and any references that follow) so the identifier is
correct and consistent; update any imports/exports or references within the same
file that rely on desitnationDir to use destinationDir instead.

---

Nitpick comments:
In `@packages/mg-ghost-api/lib/processor.js`:
- Line 83: Call to processPosts passes an unused second argument (output.users)
which the function processPosts (declared around line 49) does not accept;
remove the extraneous argument by changing the call that sets output.posts from
processPosts(input.posts, output.users) to just processPosts(input.posts), or if
user data is actually required update the processPosts function signature to
accept and use users, referencing processPosts and the output.posts assignment
so the change is applied consistently.

In `@packages/mg-wp-api/lib/processor.js`:
- Around line 1327-1332: The current for-loop processes posts sequentially
calling processPost and nulling entries, which removes the prior batched
parallelism (BATCH_SIZE) and reduces throughput; change this to process posts in
chunks using the existing BATCH_SIZE constant (or define one) and run each chunk
through Promise.all to execute processPost(posts[i], users, options, errors,
fileCache) in parallel, push all resolved results into results, and null out
processed posts to preserve memory characteristics—keep using the same variables
(posts, results, processPost, users, options, errors, fileCache) and ensure
errors from Promise.all are handled/propagated similarly to the previous
implementation.

In `@packages/mg-wp-api/test/process.test.js`:
- Around line 511-532: The tests assert.equal on the entire serialized HTML
(using processor.processContent) are too brittle; change them to verify behavior
only by either using assert.match with a regex for presence of the audio element
and attributes (e.g. style="width:100%", presence of autoplay and loop
attributes) or parse the processed HTML with linkedom/DOM parsing and assert
that the audio element has style width:100%, has attributes autoplay and/or loop
as expected and that the twitter script element has src and async attributes;
replace references to assert.equal in these test cases with assertions that
check audio[style="width:100%"], audio[autoplay], audio[loop], and
script[src][async] rather than exact serializer output (affecting the tests that
call processor.processContent and currently use assert.equal).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 73cb88c0-04cd-4a87-bc43-e865f1ef673e

📥 Commits

Reviewing files that changed from the base of the PR and between b948d36 and 3aacaa7.

⛔ Files ignored due to path filters (1)

• yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (24)

• packages/mg-blogger/lib/process.js
• packages/mg-chorus/lib/processor.js
• packages/mg-curated-export/lib/process.js
• packages/mg-ghost-api/lib/processor.js
• packages/mg-jekyll-export/lib/process.js
• packages/mg-letterdrop/lib/processor.js
• packages/mg-letterdrop/test/processor.test.js
• packages/mg-libsyn/lib/processor.js
• packages/mg-libsyn/test/processor.test.js
• packages/mg-linkfixer/lib/LinkFixer.js
• packages/mg-medium-export/lib/process-post.js
• packages/mg-medium-export/lib/process-profile.js
• packages/mg-medium-export/lib/process.js
• packages/mg-squarespace-xml/lib/process.js
• packages/mg-substack/lib/process.js
• packages/mg-utils/README.md
• packages/mg-utils/examples/bench-memory.ts
• packages/mg-utils/package.json
• packages/mg-utils/src/lib/dom-utils.ts
• packages/mg-utils/src/test/dom-utils.test.ts
• packages/mg-wp-api/lib/processor.js
• packages/mg-wp-api/test/process.test.js
• packages/mg-wp-api/utils/users-html-to-json.js
• packages/mg-wp-xml/lib/process.js