Add replaceAllMetaTags function to replace-meta-tag util
#2644
+40
−19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe pull request introduces a new utility function, Changes
Sequence Diagram(s)sequenceDiagram
participant M as Middleware
participant U as replaceAllMetaTags
participant R as replaceMetaTag
M->>U: Invoke replaceAllMetaTags(indexFileText, pageTitle, pageDescription, pageImageUrl)
loop For each meta tag specification
U->>R: Call replaceMetaTag(text, currentSpec)
R-->>U: Return updated text
end
U-->>M: Return final updated text
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Test Results 1 files ±0 1 suites ±0 6m 20s ⏱️ -15s Results for commit 9ca1074. ± Comparison against base commit e9ad543. This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
VasylMarchuk
changed the title
replaceAllMetaTags function to replace-meta-tags utilreplaceAllMetaTags function to replace-meta-tag util
❌ 1 Tests Failed:
View the full list of 1 ❄️ flaky tests
To view more test analytics, go to the Test Analytics Dashboard |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/utils/replace-meta-tag.js (1)
34-44: Add parameter validation and error handling.While the implementation is clean and efficient, consider adding validation for required parameters and error handling for invalid input.
export function replaceAllMetaTags(text, pageTitle, pageDescription, pageImageUrl) { + if (!text) { + throw new Error('text parameter is required'); + } + + // Use empty string as fallback for optional parameters + const title = pageTitle || ''; + const description = pageDescription || ''; + const imageUrl = pageImageUrl || ''; + return [ - ['name', 'description', pageDescription], // <meta name="description" content="..."> - ['property', 'og:title', pageTitle], - ['property', 'og:description', pageDescription], - ['property', 'og:image', pageImageUrl], - ['name', 'twitter:title', pageTitle], - ['name', 'twitter:description', pageDescription], - ['name', 'twitter:image', pageImageUrl], + ['name', 'description', description], // <meta name="description" content="..."> + ['property', 'og:title', title], + ['property', 'og:description', description], + ['property', 'og:image', imageUrl], + ['name', 'twitter:title', title], + ['name', 'twitter:description', description], + ['name', 'twitter:image', imageUrl], ].reduce((text, args) => replaceMetaTag(text, ...args), text); }tests/unit/utils/replace-meta-tag-test.ts (1)
54-67: Add test cases for edge cases.While the basic functionality is tested, consider adding test cases for:
- Empty or invalid input parameters
- Missing meta tags in input text
- Malformed meta tags
module('replaceAllMetaTags', function () { test('it overwrites content of all specified meta tags in passed text', function (assert) { assert.strictEqual( replaceAllMetaTags( '<meta property="og:image" content="old image"><meta property="og:title" content="old title"><meta property="og:description" content="old description"><meta name="twitter:image" content="old twitter image">', 'new title', 'new description', 'new image url', ), '<meta property="og:image" content="new image url"><meta property="og:title" content="new title"><meta property="og:description" content="new description"><meta name="twitter:image" content="new image url">', 'all old content is replaced with new content', ); }); + + test('it handles empty input parameters gracefully', function (assert) { + assert.strictEqual( + replaceAllMetaTags('<meta property="og:title" content="old title">', '', '', ''), + '<meta property="og:title" content="">', + 'empty parameters result in empty content', + ); + }); + + test('it handles missing meta tags in input text', function (assert) { + assert.strictEqual( + replaceAllMetaTags('<div>no meta tags here</div>', 'title', 'desc', 'img'), + '<div>no meta tags here</div>', + 'text without meta tags remains unchanged', + ); + }); + + test('it handles malformed meta tags', function (assert) { + assert.strictEqual( + replaceAllMetaTags('<meta property="og:title" content="old title" />', 'new title', '', ''), + '<meta property="og:title" content="new title" />', + 'handles self-closing meta tags', + ); + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/utils/replace-meta-tag.js(1 hunks)middleware.js(2 hunks)tests/unit/utils/replace-meta-tag-test.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
app/utils/replace-meta-tag.js (1)
25-33: LGTM! Documentation is clear and follows JSDoc format.The documentation clearly describes the function's purpose, parameters, and return value.
VasylMarchuk
force-pushed
the
add-replace-all-meta-tags-util
branch
from
3db00d6 to
9ca1074
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/utils/replace-meta-tag-test.ts (1)
54-67: Add test cases for edge cases.While the current test case verifies the basic functionality, consider adding test cases for:
- Missing meta tags
- Invalid meta tags
- Empty values for title, description, or image URL
- Malformed HTML
Here's an example of additional test cases:
module('replaceAllMetaTags', function () { test('it overwrites content of all specified meta tags in passed text', function (assert) { assert.strictEqual( replaceAllMetaTags( '<meta property="og:image" content="old image"><meta property="og:title" content="old title"><meta property="og:description" content="old description"><meta name="twitter:image" content="old twitter image">', 'new title', 'new description', 'new image url', ), '<meta property="og:image" content="new image url"><meta property="og:title" content="new title"><meta property="og:description" content="new description"><meta name="twitter:image" content="new image url">', 'all old content is replaced with new content', ); }); + + test('it handles missing meta tags gracefully', function (assert) { + assert.strictEqual( + replaceAllMetaTags( + '<meta property="og:title" content="old title">', + 'new title', + 'new description', + 'new image url', + ), + '<meta property="og:title" content="new title">', + 'only updates existing meta tags', + ); + }); + + test('it handles empty values gracefully', function (assert) { + assert.strictEqual( + replaceAllMetaTags( + '<meta property="og:title" content="old title"><meta property="og:description" content="old description">', + '', + '', + '', + ), + '<meta property="og:title" content=""><meta property="og:description" content="">', + 'updates meta tags with empty values', + ); + }); + + test('it handles malformed HTML gracefully', function (assert) { + assert.strictEqual( + replaceAllMetaTags( + '<meta property="og:title" content="old title"<meta property="og:description">', + 'new title', + 'new description', + 'new image url', + ), + '<meta property="og:title" content="new title"<meta property="og:description">', + 'updates valid meta tags in malformed HTML', + ); + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/utils/replace-meta-tag.js(1 hunks)middleware.js(2 hunks)tests/unit/utils/replace-meta-tag-test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/utils/replace-meta-tag.js
🔇 Additional comments (4)
tests/unit/utils/replace-meta-tag-test.ts (2)
1-1: LGTM!The import statement correctly imports both functions.
5-52: LGTM!The existing test cases for
replaceMetaTagare comprehensive and cover important edge cases.middleware.js (2)
19-19: LGTM!The import statement correctly imports the new function.
133-134: LGTM!The meta tag replacement logic has been simplified and improved by using
replaceAllMetaTags. The parameter order is correct, and the code is more maintainable.
Bundle ReportChanges will increase total bundle size by 278 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: client-array-pushAssets Changed:
Files in
|
rohitpaulk
approved these changes
Feb 11, 2025
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #2607
Brief
In preparation to adding pre-rendering for user & concept pages, this extracts the logic to replace all required OG meta tags from our vercel middleware into
app/utils/replace-meta-tag.js, so that it can be reused in prerender functions.Details
Tests included.
Checklist
[percy]in the message to trigger)Summary by CodeRabbit