Skip to content

OUT-2310, OUT-2287, OUT-2246 | client home crashed with undefined on replaceAll - #105

Merged
arpandhakal merged 1 commit into
mainfrom
OUT-2310
Sep 5, 2025
Merged

OUT-2310, OUT-2287, OUT-2246 | client home crashed with undefined on replaceAll#105
arpandhakal merged 1 commit into
mainfrom
OUT-2310

Conversation

@arpandhakal

Copy link
Copy Markdown
Collaborator

The main changes are

  • fixed javascript methods on undefined values like title and contents. Added some checks before proceeding with event handlers too.

…e and contents. Added some checks before proceeding with event handlers too
@arpandhakal arpandhakal self-assigned this Sep 5, 2025
@vercel

vercel Bot commented Sep 5, 2025

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
client-home Ready Ready Preview Comment Sep 5, 2025 10:03am

@rrojan rrojan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, apart from requested changes

Comment on lines 219 to +220
replaceCustomLabelsWithPlaceholders(
defaultState.replaceAll(' ', ''),
defaultState?.replaceAll(' ', ''),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does replaceCustomLabelsWithPlaceholders handle undefined as a first param? If not, we should default to an empty string:

Suggested change
replaceCustomLabelsWithPlaceholders(
defaultState.replaceAll(' ', ''),
defaultState?.replaceAll(' ', ''),
replaceCustomLabelsWithPlaceholders(
defaultState?.replaceAll(' ', '') ?? '',

Comment on lines +42 to +43
item
.toLowerCase()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
item
.toLowerCase()
item
?.toLowerCase()

@arpandhakal
arpandhakal merged commit 2e18d35 into main Sep 5, 2025
3 checks passed
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.

2 participants