|
| 1 | +# GitHub Copilot Instructions: Ember Octane & Project Best Practices |
| 2 | + |
| 3 | +This document provides guidelines for code changes within our EmberJS project. GitHub Copilot should use these instructions to provide relevant suggestions, identify potential issues, and assist in adhering to our project's conventions, Ember Octane best practices, and overall code quality. |
| 4 | + |
| 5 | +**Prioritize reusability, testability, and maintainability in all code changes.** |
| 6 | + |
| 7 | +## Guidelines by File Type |
| 8 | + |
| 9 | +### For Files in the `changelog` Directory |
| 10 | + |
| 11 | +**Instruction:** Ensure `changelog` entries for enterprise-only changes explicitly indicate "enterprise" in their description. |
| 12 | + |
| 13 | +* **DON'T:** `ui: descriptive text` |
| 14 | +* **DO:** `ui (enterprise): descriptive text` |
| 15 | + |
| 16 | +### For Files in the `components` Directory |
| 17 | + |
| 18 | +**Instruction:** Do not use `@tracked` on properties that are immutable or do not change throughout the component's lifecycle. `@tracked` is for reactive state. |
| 19 | + |
| 20 | +* **DON'T:** `@secret={{concat "role/" @model.id}}` (Here, `@secret` is an argument, not a reactive property of the component itself.) |
| 21 | +* **DO:** `@tracked myReactiveProperty = "initialValue";` (Use `@tracked` for component-internal state that can change.) |
| 22 | + |
| 23 | +**Instruction:** Component class names should always match their corresponding file names for clarity and Ember's convention. |
| 24 | + |
| 25 | +**Instruction:** Co-locate `.hbs` templates in the same directory as their corresponding `.js` or `.ts` component files (e.g., `app/components/my-component/my-component.js` and `app/components/my-component/my-component.hbs`). |
| 26 | + |
| 27 | +**Instruction:** Avoid unnecessary quotation marks around dynamic data attributes in templates. |
| 28 | + |
| 29 | +* **DON'T:** `data-test-namespace-link="{{option.label}}"` |
| 30 | +* **DO:** `data-test-namespace-link={{option.label}}` |
| 31 | + |
| 32 | +### For Files in the `models` Directory |
| 33 | + |
| 34 | +**Instruction:** Use single-line syntax for `@attr` declarations if they have one or fewer keys in their options object. |
| 35 | + |
| 36 | +* **DON'T:** |
| 37 | + ```javascript |
| 38 | + @attr('string', { |
| 39 | + label: 'Client ID', |
| 40 | + }) clientId; |
| 41 | + ``` |
| 42 | +* **DO:** `@attr('string', { label: 'Client ID' }) clientId;` |
| 43 | + |
| 44 | +**Instruction:** Do not add extra blank lines between consecutive single-line `@attr` declarations to maintain compactness. |
| 45 | + |
| 46 | +* **DON'T:** |
| 47 | + ```javascript |
| 48 | + @attr('string', { label: 'Client ID' }) clientId; |
| 49 | +
|
| 50 | + @attr('string', { label: 'Client Secret' }) clientSecret; |
| 51 | + ``` |
| 52 | +* **DO:** |
| 53 | + ```javascript |
| 54 | + @attr('string', { label: 'Client ID' }) clientId; |
| 55 | + @attr('string', { label: 'Client Secret' }) clientSecret; |
| 56 | + ``` |
| 57 | +
|
| 58 | +### For Files Ending in `.js` or `.ts` |
| 59 | +
|
| 60 | +**Instruction:** Include JSDoc-style documentation (e.g., `/** @module ComponentName */` for modules, `/** ... */` for classes, methods, and properties) for all new files, especially components, helpers, services, and complex functions. |
| 61 | +
|
| 62 | +**Instruction:** Remove all unused code, imports, or constants to keep the codebase clean and efficient. |
| 63 | +
|
| 64 | +**Instruction:** Avoid using `console.error`. Instead, use Ember's `assert` for runtime checks or `debug` for logging during development. |
| 65 | + |
| 66 | +**Instruction:** Ensure all comments are up-to-date and accurately reflect the current code logic. Remove or update outdated comments. |
| 67 | + |
| 68 | +**Instruction:** Avoid using the `any` type in TypeScript files. Strive for strict typing to improve code predictability and maintainability. |
| 69 | + |
| 70 | +**Instruction:** Only use `@task` from `ember-concurrency` when you specifically need to leverage its advanced features like `isRunning`, `perform`, `cancel`, or task modifiers. Otherwise, prefer standard `async`/`await` with `try`/`catch` blocks for asynchronous operations. |
| 71 | + |
| 72 | +**Instruction:** If `setTimeout` is used, consider whether `requestAnimationFrame`, `@tracked` properties with getters, or other reactive Ember patterns might be more appropriate. `setTimeout` can lead to testing complexities and event loop management issues. |
| 73 | + |
| 74 | +**Instruction:** Do not use `new Date()` directly. To ensure consistent UTC date calculations across all environments, use `Date.UTC(...)` and `getUTCFullYear()`, `getUTCMonth()`, etc. for date manipulation. |
| 75 | + |
| 76 | +### For Files in the `tests` Directory |
| 77 | + |
| 78 | +**Instruction:** Replace `setTimeout` with Ember's `run.later` method within tests for better control over the Ember runloop. |
| 79 | +
|
| 80 | +**Instruction:** When using `ember-cli-mirage`, place all `this.server` setup steps at the top of the `test` or `module` block for clarity and consistency. |
| 81 | +
|
| 82 | +**Instruction:** Avoid using `assert.ok()`. Prefer `assert.true()` or `assert.false()` for clearer boolean assertions. |
| 83 | +
|
| 84 | +**Instruction:** Provide clear, descriptive messages for all assertions to aid in debugging test failures. |
| 85 | +
|
| 86 | +* **DON'T:** `assert.dom(GENERAL.messageError).hasText('Error');` |
| 87 | +* **DO:** `assert.dom(GENERAL.messageError).hasText('Error', "Verify that the error message is displayed correctly.");` |
| 88 | + |
| 89 | +**Instruction:** Use `data-test-*` selectors for all DOM interactions within tests. This decouples tests from presentational styles and markup changes. |
| 90 | + |
| 91 | +### For Files Ending in `.hbs` (Handlebars Templates) |
| 92 | + |
| 93 | +**Instruction:** Avoid using the `.length` property in logical operators within `{{if}}` or `{{unless}}` helpers for arrays. Empty arrays are already considered falsy in Ember templates. |
| 94 | + |
| 95 | +* **DON'T:** `{{#if (gt this.model.allowed_roles.length 0)}}` |
| 96 | +* **DO:** `{{#if this.model.allowed_roles}}` |
| 97 | +
|
| 98 | +**Instruction:** Avoid using the `{{concat}}` helper. Prefer string interpolation directly within attributes for cleaner syntax. |
| 99 | +
|
| 100 | +* **DON'T:** `@secret={{concat "role/" @model.id}}` |
| 101 | +* **DO:** `@secret="role/{{@model.id}}"` |
| 102 | + |
| 103 | +**Instruction:** Ensure that any links leading to `vaut/docs` are rendered using `Hds::Link::Inline` and not a standard `<button>`. |
| 104 | + |
| 105 | +**Instruction:** Do not use unnecessary quotation marks around double curly brace expressions when passing dynamic values to component arguments. |
| 106 | + |
| 107 | +* **DON'T:** `@user="{{@model.name}}"` |
| 108 | +* **DO:** `@user={{@model.name}}` |
| 109 | +
|
| 110 | +**Instruction:** For `selected` attributes, the passed-in property should almost always be dynamic. If a static value (e.g., `true` or a fixed string) is used, suggest a review to confirm it is intentional and correct. |
| 111 | +
|
| 112 | +* **DON'T:** `selected="true"` |
| 113 | +* **DO:** `selected={{eq this.selectedAuthMethod type}}` |
| 114 | + |
| 115 | +**Instruction:** If a conditional wraps an entire element, refactor it so the conditional wraps only the dynamic *content* within the element, improving readability and reducing HTML boilerplate. |
| 116 | + |
| 117 | +* **DON'T:** |
| 118 | + ```handlebars |
| 119 | + {{#if this.version.isEnterprise}} |
| 120 | + <PH.Title>Enterprise things</PH.Title> |
| 121 | + {{else}} |
| 122 | + <PH.Title>Community things</PH.Title> |
| 123 | + {{/if}} |
| 124 | + ``` |
| 125 | +* **DO:** `<PH.Title>{{if this.version.isEnterprise "Enterprise things" "Community things"}}</PH.Title>` |
| 126 | +
|
| 127 | +**Instruction:** Avoid using the `style` attribute for inline styling. Define and use CSS classes within `.scss` files instead. |
| 128 | +
|
| 129 | +* **DON'T:** (This is a corrected example you provided, but the original was missing the 'style' attribute to demonstrate the 'DONT'. Assuming your intention was to show moving test selectors.) |
| 130 | + |
| 131 | +* **DO:** (The example you gave previously for 'Correct' for `style` was already good, I'll clarify the `data-test` rule below. For `style` attribute, the `DO` would look like this if you had classes defined instead of inline styles.) |
| 132 | +
|
| 133 | +**Instruction:** Place `data-test-*` selectors as the *last* attribute on an HTML element for consistency and ease of parsing. |
| 134 | +
|
| 135 | +* **DON'T:** (The example you provided for this `data-test` rule appears to be the same as the previous one, where the `data-test-save` was inside the attributes, but not necessarily last. I'll modify the `DON'T` to make it more explicit for `data-test` position.) |
| 136 | + ```handlebars |
| 137 | + <Hds::Button |
| 138 | + data-test-save |
| 139 | + @text={{if (eq @mountType "secret") "Enable engine" "Enable method"}} |
| 140 | + type="submit" |
| 141 | + /> |
| 142 | + ``` |
| 143 | +* **DO:** |
| 144 | + ```handlebars |
| 145 | + <Hds::Button |
| 146 | + @text={{if (eq @mountType "secret") "Enable engine" "Enable method"}} |
| 147 | + type="submit" |
| 148 | + data-test-save |
| 149 | + /> |
| 150 | + ``` |
| 151 | +
|
| 152 | +### For Files Ending in `.scss` |
| 153 | +
|
| 154 | +**Instruction:** Avoid using `z-index`. Instead, manage element stacking order by adjusting their natural order in the template (DOM structure). |
| 155 | +
|
| 156 | +* **DON'T:** |
| 157 | + ```css |
| 158 | + .modal { |
| 159 | + position: absolute; |
| 160 | + z-index: 10; |
| 161 | + } |
| 162 | + ``` |
| 163 | +* **DO:** (This instruction implies the HTML change, so the SCSS `DO` is the absence of `z-index`). |
| 164 | + ```css |
| 165 | + /* Adjust HTML structure to control stacking instead of z-index */ |
| 166 | + .modal { |
| 167 | + position: absolute; |
| 168 | + /* no z-index needed if stacking context is managed by DOM order */ |
| 169 | + } |
| 170 | + ``` |
| 171 | +
|
| 172 | +**Instruction:** Avoid using `!important`. Instead, achieve desired style overrides by increasing CSS specificity (e.g., by targeting elements with more specific selectors or by using component-scoped styles). |
| 173 | +
|
| 174 | +* **DON'T:** |
| 175 | + ```css |
| 176 | + .button { |
| 177 | + color: red !important; |
| 178 | + } |
| 179 | + ``` |
| 180 | +* **DO:** |
| 181 | + ```css |
| 182 | + .namespace-picker .button { /* More specific selector */ |
| 183 | + color: red; |
| 184 | + } |
| 185 | + ``` |
| 186 | +
|
| 187 | +### For Changes to `package.json` |
| 188 | +
|
| 189 | +**Instruction:** Ensure `package.json` changes are independent and minimal, relating only to the immediate feature or bug fix. The `yarn.lock` file is the only expected co-change. |
| 190 | +
|
| 191 | +**Instruction:** When adding or modifying dependencies, prefer pinning exact versions or using the tilde (`~`) for patch-level updates. Avoid using the caret (`^`) operator for dependency versions. |
| 192 | +
|
| 193 | +* **DON'T:** `"ansi-html": "^0.0.8"` |
| 194 | +* **DO:** `"ansi-html": "~0.0.8"` or `"ansi-html": "0.0.8"` |
| 195 | +
|
| 196 | +**Instruction:** Dependencies within the `resolutions` block must always be pinned to an exact version (no `~` or `^`). |
| 197 | +
|
| 198 | +## General Guidelines |
| 199 | +
|
| 200 | +**Instruction:** Replace deprecated `Route.extend`, `Model.extend`, or `Component.extend` syntax with their modern Ember Octane class-based equivalents. |
| 201 | +
|
| 202 | +**Instruction:** Place comments directly above the code lines or blocks they describe, not below or interspersed. |
| 203 | +
|
| 204 | +**Instruction:** Use sentence case for all titles (e.g., for component arguments like `@title`). |
| 205 | +
|
| 206 | +* **DON'T:** `@title="Upload Users'sProfile"` |
| 207 | +* **DO:** `@title="Upload user's profile"` |
| 208 | +
|
| 209 | +**Instruction:** Ensure all subtitles and descriptive text follow proper grammar rules, including ending sentences with periods. |
| 210 | +
|
| 211 | +**Instruction:** All new components (but not tests) should be co-located within their logical structure inside the `app/components` directory. |
| 212 | +
|
| 213 | +**Instruction:** Adhere to Ember's built-in best practices for code structure and syntax, consulting the official Ember documentation for the specific version in use. |
| 214 | +
|
| 215 | +**Instruction:** Prioritize creating reusable and maintainable code. Avoid overly complex or one-off component/route implementations without strong justification. |
| 216 | +
|
| 217 | +**Instruction:** When referring to KV secret engines, use the precise terms "KVv2" or "KVv1". When space allows, prefer spelling out "KV version 2" or "KV version 1". |
| 218 | +
|
| 219 | +* **DON'T:** `KV v2` |
| 220 | +* **DO:** `KV version 2` or `KVv2` (depending on space/context) |
| 221 | +
|
| 222 | +## Debugging Tips (Informational, not for Copilot to "enforce") |
| 223 | +
|
| 224 | +* **Tip:** When using `{{debugger}}` inside a template, you can inspect values in the browser console. |
| 225 | + * **Example:** |
| 226 | + ```handlebars |
| 227 | + {{#each this.items as |item|}} |
| 228 | + {{debugger}} |
| 229 | + {{/each}} |
| 230 | + ``` |
| 231 | + * **In the console:** |
| 232 | + * Use `get('item.name')` to inspect an item's property. |
| 233 | + * Use `context` to explore the current rendering context. |
0 commit comments