feat(Link):添加 AddToHead 参数
#7092
Closed
Conversation
This file contains hidden or 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
|
Thanks for your PR, @AiYuZhen. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Contributor
Reviewer's GuideAdds an IsAddToHead flag to the Link component enabling optional insertion of tags into the document head via a JS module, avoids duplicate tags, and updates unit tests. Sequence diagram for Link component rendering with IsAddToHead=truesequenceDiagram
participant LinkComponent
participant Browser
participant JSModule
LinkComponent->>Browser: Render (firstRender)
LinkComponent->>JSModule: Invoke init({Href, Rel})
JSModule->>Browser: Check for existing <link> in head
alt Link not found
JSModule->>Browser: Append new <link> to head
else Link found
JSModule->>Browser: Do nothing
end
Class diagram for updated Link componentclassDiagram
class Link {
+string? Version
+bool IsAddToHead
+IVersionService VersionService
+string GetHref()
+Task OnAfterRenderAsync(bool firstRender)
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider renaming IsAddToHead to a name like AddToHead or AppendToHead to better follow .NET boolean property naming conventions.
- Add disposal or parameter‐change logic to remove or update injected link tags when the component is unmounted or its parameters change to prevent stale entries.
- In the JavaScript init function, use the URL API (e.g. new URL(href, document.baseURI)) instead of string concatenation for more robust href comparisons and to support relative URLs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider renaming IsAddToHead to a name like AddToHead or AppendToHead to better follow .NET boolean property naming conventions.
- Add disposal or parameter‐change logic to remove or update injected link tags when the component is unmounted or its parameters change to prevent stale entries.
- In the JavaScript init function, use the URL API (e.g. new URL(href, document.baseURI)) instead of string concatenation for more robust href comparisons and to support relative URLs.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/HtmlTag/Link.razor.js:2-3` </location>
<code_context>
+export function init(options) {
+ const href = options.href;
+ const rel = options.rel;
+ const links = document.head.getElementsByTagName("link");
+ // 遍历所有link元素
</code_context>
<issue_to_address>
**issue:** Consider normalizing option property casing for consistency.
The C# object uses 'Href' and 'Rel', while the JS expects 'href' and 'rel'. Please align property names to prevent undefined values.
</issue_to_address>
### Comment 2
<location> `src/BootstrapBlazor/Components/HtmlTag/Link.razor.js:8` </location>
<code_context>
+ // 遍历所有link元素
+ for (let i = 0; i < links.length; i++) {
+ const hlink = links[i];
+ var nhref = hlink.baseURI + href;
+ if (hlink.href == nhref && hlink.rel == rel) {
+ return;
</code_context>
<issue_to_address>
**issue (bug_risk):** The construction of nhref may not match the actual href attribute value.
Directly concatenating baseURI and href can lead to incorrect URLs if href is absolute or already includes the base URI. Use proper URL resolution or compare hlink.href and href directly.
</issue_to_address>
### Comment 3
<location> `src/BootstrapBlazor/Components/HtmlTag/Link.razor.js:2` </location>
<code_context>
const href = options.href;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))
```suggestion
const {href} = options;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 4
<location> `src/BootstrapBlazor/Components/HtmlTag/Link.razor.js:3` </location>
<code_context>
const rel = options.rel;
</code_context>
<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))
```suggestion
const {rel} = options;
```
<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.
From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>
### Comment 5
<location> `src/BootstrapBlazor/Components/HtmlTag/Link.razor.js:8` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))
<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.
From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
AiYuZhen
changed the title
IsAddToHead 参数，支持将 link 标签添加到 head 元素中。IsAddToHead 参数
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7092 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 745 746 +1
Lines 32556 32568 +12
Branches 4512 4514 +2
=========================================
+ Hits 32556 32568 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
通过 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
#7091
Link issues
fixes #{issue number}
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Enable Link components to inject tags into the page head through a new IsAddToHead parameter and a custom JS interop module, while preserving existing rendering behavior when the feature is not enabled.
New Features:
Enhancements:
Tests: