|
| 1 | +--- |
| 2 | +mode: edit |
| 3 | +--- |
| 4 | + |
| 5 | +# Instructions for Filling in an API Review Issue |
| 6 | + |
| 7 | +## Prerequisites |
| 8 | + |
| 9 | +Before you begin filling out an API review issue, gather the following information: |
| 10 | + |
| 11 | +* **Original issue** - The GitHub issue or feature request that prompted this API change |
| 12 | +* **GitHub commit(s) containing the implementation** (if any) - Links to specific commits where the API was implemented |
| 13 | + * **Pay special attention to `PublicAPI.Unshipped.txt` files** - These files contain API changes and are crucial for the review |
| 14 | + |
| 15 | +**ASK FOR CONFIRMATION**: Make a check-list of all the information (issue and commit). Mark the items that you have gathered. If any item is missing, ask the author to provide it before proceeding. |
| 16 | + |
| 17 | +## Overview |
| 18 | + |
| 19 | +The API review process ensures that new APIs follow common patterns and best practices while guiding engineers toward better API design decisions. The goal is to provide enough context for people working outside that feature area to understand what the change is about and give meaningful feedback. |
| 20 | + |
| 21 | +## Sections |
| 22 | + |
| 23 | +**DO NOT ADD ANY ADDITIONAL INFORMATION THAT IS NOT PART OF THE ORIGINAL ISSUE OR CODE CHANGES** |
| 24 | +**AT THE END OF THE DOCUMENT, FOR ALL CONTENT ON EACH SECTION, YOU MUST JUSTIFY THE SOURCE FOR THAT CONTENT USING A QUOTE FRAGMENT FROM THE ORIGINAL ISSUE OR CODE CHANGES IN THIS FORMAT <<CONTENT>>: "<<QUOTE>>"** |
| 25 | +**ASK FOR ADDITIONAL INFORMATION IF THERE IS NOT ENOUGH CONTEXT IN THE ORIGINAL ISSUE OR CODE CHANGES TO FILL IN A SECTION** |
| 26 | +**YOU CAN FILL IN ANY SECTION WITH N/A IF THERE IS NO RELEVANT INFORMATION AVAILABLE** |
| 27 | + |
| 28 | +Fill out each section following the format and guidelines below: |
| 29 | + |
| 30 | +### 1. Background and Motivation |
| 31 | + |
| 32 | +**Purpose**: Explain the purpose and value of the new API to help reviewers understand why this change is needed. |
| 33 | + |
| 34 | +**Guidelines**: |
| 35 | +- Provide a clear, concise description of the problem being solved |
| 36 | +- Explain the current limitations or gaps that this API addresses |
| 37 | +- Reference the original issue that prompted this change |
| 38 | +- Focus on the "why" rather than the "how" |
| 39 | + |
| 40 | +**Good Example**: |
| 41 | +> Previously, users were able to invoke JavaScript functions from .NET code using the `InvokeAsync` method from the `IJSRuntime` and `IJSObjectReference` interfaces. To perform any other JavaScript operation, they had to wrap it into a plain JavaScript function, deploy that function with their application, and invoke it via `InvokeAsync`. To reduce the need for such boilerplate code, we propose adding methods to the interop API to enable performing common operations directly. |
| 42 | +
|
| 43 | +**Bad Example**: |
| 44 | +> Adding a string overload for Widget.ConfigureFactory. |
| 45 | +
|
| 46 | +### 2. Proposed API |
| 47 | + |
| 48 | +**Purpose**: Provide the specific public API signature diff that you are proposing. |
| 49 | + |
| 50 | +**Requirements**: |
| 51 | +- Use **ref-assembly format** (more readable and useful for API review discussions) |
| 52 | +- Include the complete namespace and type declarations |
| 53 | +- Show additions with `+` prefix in diff format |
| 54 | +- If linking to generated ref-assembly code in a PR, that's acceptable |
| 55 | +- For areas that don't produce ref-assemblies, write out what it would look like in ref-assembly format |
| 56 | + |
| 57 | +**Format Example**: |
| 58 | +```diff |
| 59 | +namespace Microsoft.AspNetCore.Http; |
| 60 | + |
| 61 | +public static class HttpResponseWritingExtensions |
| 62 | +{ |
| 63 | ++ public Task WriteAsync(this HttpResponse response, StringBuilder builder); |
| 64 | +} |
| 65 | +``` |
| 66 | + |
| 67 | +**Complex API Example**: |
| 68 | +```diff |
| 69 | +namespace Microsoft.JSInterop |
| 70 | +{ |
| 71 | + public interface IJSRuntime |
| 72 | + { |
| 73 | ++ ValueTask<TValue> GetValueAsync<TValue>(string identifier); |
| 74 | ++ ValueTask<TValue> GetValueAsync<TValue>(string identifier, CancellationToken cancellationToken); |
| 75 | ++ ValueTask SetValueAsync<TValue>(string identifier, TValue value); |
| 76 | ++ ValueTask SetValueAsync<TValue>(string identifier, TValue value, CancellationToken cancellationToken); |
| 77 | + } |
| 78 | +} |
| 79 | +``` |
| 80 | + |
| 81 | +**Key Points**: |
| 82 | +- Include all overloads and extension methods |
| 83 | +- Show the complete type hierarchy when adding to existing interfaces |
| 84 | +- Reference the `PublicAPI.Unshipped.txt` files if available in your implementation commits |
| 85 | + |
| 86 | +### 3. Usage Examples |
| 87 | + |
| 88 | +**Purpose**: Demonstrate how the proposed API additions are meant to be consumed to validate that the API has the right shape to be functional, performant, and usable. |
| 89 | + |
| 90 | +**Guidelines**: |
| 91 | +- Provide realistic, practical code examples |
| 92 | +- Show both simple and complex usage scenarios |
| 93 | +- Include both synchronous and asynchronous variants (if applicable) |
| 94 | +- Use proper C# code block formatting |
| 95 | + |
| 96 | +**Example Format**: |
| 97 | +```csharp |
| 98 | +@inject IJSRuntime JSRuntime |
| 99 | + |
| 100 | +// Simple property access |
| 101 | +string title = await JSRuntime.GetValueAsync<string>("document.title"); |
| 102 | + |
| 103 | +// Setting values |
| 104 | +await JSRuntime.SetValueAsync("document.title", "Hello there"); |
| 105 | + |
| 106 | +// Constructor invocation |
| 107 | +IJSObjectReference chartRef = await JSRuntime.InvokeNewAsync("Chart", chartParams); |
| 108 | + |
| 109 | +// Working with object references |
| 110 | +var someChartProperty = await chartRef.GetValueAsync<int>("somePropName"); |
| 111 | +``` |
| 112 | + |
| 113 | +### 4. Alternative Designs |
| 114 | + |
| 115 | +**Purpose**: Show that you've considered other approaches and explain why the proposed design is the best option. |
| 116 | + |
| 117 | +**Include**: |
| 118 | +- Other API shapes you considered |
| 119 | +- Comparison to analogous APIs in other ecosystems and libraries |
| 120 | +- Trade-offs between different approaches |
| 121 | +- Why the proposed approach was chosen over alternatives |
| 122 | + |
| 123 | +**Example**: |
| 124 | +> We considered supporting the additional operations with only the existing `InvokeAsync` method and selecting its behavior according to what JS entity is found based on the `identifier`. However, this approach has obvious UX issues (clarity, predictability). There is also no way to differentiate, in general, between "normal" and "constructor" functions in JavaScript. |
| 125 | +
|
| 126 | +### 5. Risks |
| 127 | + |
| 128 | +**Purpose**: Identify potential issues or concerns with the proposed API. |
| 129 | + |
| 130 | +**Consider**: |
| 131 | +- Breaking changes to existing code |
| 132 | +- Performance implications or regressions |
| 133 | +- Security concerns |
| 134 | +- Compatibility issues |
| 135 | +- Potential for misuse |
| 136 | +- Impact on existing patterns or conventions |
| 137 | + |
| 138 | +**Example**: |
| 139 | +> The added interface methods have default implementations (which throw `NotImplementedException`) to avoid breaking builds of their implementors. A possible criticism of the feature is that streamlining interop in this manner might motivate misuse of inefficient interop calls. |
| 140 | +
|
| 141 | +## Quality Checklist |
| 142 | + |
| 143 | +Before marking your issue as `api-ready-for-review`, ensure: |
| 144 | + |
| 145 | +- [ ] **Clear description**: Includes a short description that will help reviewers not familiar with this area |
| 146 | +- [ ] **Complete API specification**: All API changes are in ref-assembly format |
| 147 | +- [ ] **Implementation reference**: Links to commits containing the implementation, especially `PublicAPI.Unshipped.txt` files |
| 148 | +- [ ] **Adequate context**: Larger changes have more explanation and context |
| 149 | +- [ ] **Usage examples**: Realistic code examples that demonstrate the API's intended use |
| 150 | +- [ ] **Risk assessment**: Potential issues and breaking changes are identified |
| 151 | +- [ ] **Champion identified**: Someone is assigned to champion this change in the API review meeting |
| 152 | + |
| 153 | +## Process Notes |
| 154 | + |
| 155 | +1. **Label progression**: Issues move from `api-suggestion` → `api-ready-for-review` → `api-approved` |
| 156 | +2. **Team notification**: Notify @asp-net-api-reviews team when marking as `api-ready-for-review` |
| 157 | +3. **Meeting attendance**: If your API is scheduled for review, you must have a representative in the meeting |
| 158 | +4. **Implementation changes**: If changes to the original proposal are required during implementation, the review becomes obsolete and the process starts over |
| 159 | + |
| 160 | +## Reference Resources |
| 161 | + |
| 162 | +- [API Review Process Documentation](https://github.com/dotnet/aspnetcore/blob/main/docs/APIReviewProcess.md) |
| 163 | +- [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/framework-design-guidelines-digest.md) |
| 164 | +- [API Review Principles](https://github.com/dotnet/aspnetcore/blob/main/docs/APIReviewPrinciples.md) |
| 165 | +- [Pending API Reviews](https://aka.ms/aspnet/apireviews) |
| 166 | + |
| 167 | +## Template Structure |
| 168 | + |
| 169 | +```markdown |
| 170 | +## Background and Motivation |
| 171 | + |
| 172 | +<!-- |
| 173 | +We welcome API proposals! We have a process to evaluate the value and shape of new API. There is an overview of our process [here](https://github.com/dotnet/aspnetcore/blob/main/docs/APIReviewProcess.md). This template will help us gather the information we need to start the review process. |
| 174 | +First, please describe the purpose and value of the new API here. |
| 175 | +--> |
| 176 | + |
| 177 | +## Proposed API |
| 178 | + |
| 179 | +<!-- |
| 180 | +Please provide the specific public API signature diff that you are proposing. For example: |
| 181 | +```diff |
| 182 | +namespace Microsoft.AspNetCore.Http; |
| 183 | + |
| 184 | +public static class HttpResponseWritingExtensions |
| 185 | +{ |
| 186 | ++ public Task WriteAsync(this HttpResponse response, StringBuilder builder); |
| 187 | +} |
| 188 | +``` |
| 189 | +You may find the [Framework Design Guidelines](https://github.com/dotnet/runtime/blob/master/docs/coding-guidelines/framework-design-guidelines-digest.md) helpful. |
| 190 | +--> |
| 191 | + |
| 192 | +## Usage Examples |
| 193 | + |
| 194 | +<!-- |
| 195 | +Please provide code examples that highlight how the proposed API additions are meant to be consumed. |
| 196 | +This will help suggest whether the API has the right shape to be functional, performant and useable. |
| 197 | +You can use code blocks like this: |
| 198 | +```csharp |
| 199 | +// some lines of code here |
| 200 | +``` |
| 201 | +--> |
| 202 | + |
| 203 | +## Alternative Designs |
| 204 | + |
| 205 | +<!-- |
| 206 | +Were there other options you considered, such as alternative API shapes? |
| 207 | +How does this compare to analogous APIs in other ecosystems and libraries? |
| 208 | +--> |
| 209 | + |
| 210 | +## Risks |
| 211 | + |
| 212 | +<!-- |
| 213 | +Please mention any risks that to your knowledge the API proposal might entail, such as breaking changes, performance regressions, etc. |
| 214 | +--> |
| 215 | +``` |
0 commit comments