Skip to content

mobile: fix push notif rendering#5857

Merged
latter-bolden merged 5 commits into
developfrom
lb/notif-text-content
May 21, 2026
Merged

mobile: fix push notif rendering#5857
latter-bolden merged 5 commits into
developfrom
lb/notif-text-content

Conversation

@latter-bolden
Copy link
Copy Markdown
Member

@latter-bolden latter-bolden commented May 20, 2026

The problem here was #5839 adding a new markdown parsing dependency that crashes when run from the scripts bundle — our minimalist JS target that gets run natively on iOS via JavaScriptCore.

The runtime path we use from scripts is getTextContent. That helper is contained in a module that requires extractTables, which expects the DOM to be available at runtime and crashes if it's not.

I think 930b2fa alone fixes the immediate problem by changing the esbuild conditions to target RN. However, respecting that is opt-in at the module level so simply configuring the build command doesn't prevent similar issues in the future.

The other commits add a new postNotificationText helper that's intentionally limited in what it imports and is placed within the scripts module itself. I don't love having another "turn raw message into text string" helper, but I think the alternative is a thorny disentangle of postContent.ts that might regress down the line.

Implementation wise, the new helper produced identical output against post fixtures and has seemed fine anecdotally in the Preview App. However, if we're concerned about the added helper, we can leave it out and exclusively merge the esbuild fix.

Fixes TLON-5843

@dnbrwstr dnbrwstr mentioned this pull request May 20, 2026
@latter-bolden latter-bolden marked this pull request as ready for review May 20, 2026 22:19
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@linear
Copy link
Copy Markdown

linear Bot commented May 20, 2026

TLON-5843

@jamesacklin
Copy link
Copy Markdown
Member

@claude review once

Comment thread packages/scripts/src/postNotificationText.ts Outdated
Copy link
Copy Markdown
Contributor

@dnbrwstr dnbrwstr left a comment

Choose a reason for hiding this comment

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

Lgtm

@latter-bolden latter-bolden merged commit a8173b0 into develop May 21, 2026
4 checks passed
@latter-bolden latter-bolden deleted the lb/notif-text-content branch May 21, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants