-
Notifications
You must be signed in to change notification settings - Fork 4
Changes from alive #68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces changes focused on handling WordPress media library access and improving component error handling in the embedded gateway. The changes appear to address issues with URL routing for WordPress content files and add defensive checks for component resolution.
Key Changes:
- Added URL filtering logic to preserve direct access to WordPress media library files
- Enhanced component validation with explicit error messaging for missing components
- Removed data attributes from elements before rendering components
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| wp-react-lib/src/util/index.js | Added bypass for /wp/wp-content URLs to preserve media library access; added debugger statement |
| wp-react-lib/src/embedded/EmbeddedGateway.jsx | Enhanced component validation with error display for missing components; added cleanup of data attributes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| export const replaceHTMLinks = (html, locale) => { | ||
| debugger; |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The debugger statement should be removed before merging to production. This will cause the browser to pause execution in development tools, which is not intended for production code.
| debugger; |
| } | ||
| //ensuring access to media library files | ||
| if (pathname.startsWith("/wp/wp-content")){ | ||
| return url |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon at the end of the return statement. While JavaScript has automatic semicolon insertion, this is inconsistent with the style used elsewhere in the file (e.g., line 17).
| return url | |
| return url; |
|
|
||
| if (component) { | ||
| if (component != null && getComponent(component) === null) { | ||
| element.innerHTML = "<h1>Data Viz Error </h1><h4>Component<i> " + component + "</i> not found</h4><br>" |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting innerHTML with unsanitized component name could introduce XSS vulnerabilities if the component variable contains malicious content. Consider using textContent or a safer DOM manipulation method, or sanitize the component value before insertion.
| element.innerHTML = "<h1>Data Viz Error </h1><h4>Component<i> " + component + "</i> not found</h4><br>" | |
| // Safely create error message elements | |
| // Remove all children | |
| while (element.firstChild) { | |
| element.removeChild(element.firstChild); | |
| } | |
| const h1 = document.createElement("h1"); | |
| h1.textContent = "Data Viz Error"; | |
| element.appendChild(h1); | |
| const h4 = document.createElement("h4"); | |
| h4.textContent = "Component "; | |
| const i = document.createElement("i"); | |
| i.textContent = component; | |
| h4.appendChild(i); | |
| h4.appendChild(document.createTextNode(" not found")); | |
| element.appendChild(h4); | |
| element.appendChild(document.createElement("br")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
| } | ||
|
|
||
| if (component) { | ||
| if (component != null && getComponent(component) === null) { |
Copilot
AI
Dec 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition checks both component != null and getComponent(component) === null, but this calls getComponent twice (once here and again on line 50). Consider storing the result in a variable to avoid redundant calls.
|
@sdimunzio You can add a changeset we release it NPM. |
No description provided.