diff --git a/.github/instructions/generic/ember_js.instructions.md b/.github/instructions/generic/ember_js.instructions.md index b7279219ad..aedac5984d 100644 --- a/.github/instructions/generic/ember_js.instructions.md +++ b/.github/instructions/generic/ember_js.instructions.md @@ -20,6 +20,7 @@ This document provides JavaScript and TypeScript coding standards for HashiCorp - Remove all unused imports, variables, and functions before committing - Place comments directly above the code they describe, not inline or below - Update comments when code changes to maintain accuracy +- Only export functions, classes, or variables that are used by other files. If a function is only used within the same file, keep it private (no export). This reduces API surface area and improves maintainability. ## TypeScript Guidelines - **File Naming**: All new files should use `.ts` extension instead of `.js` @@ -36,6 +37,28 @@ This document provides JavaScript and TypeScript coding standards for HashiCorp - Co-locate component templates (`.hbs`) with their TypeScript files (`.ts` preferred over `.js`) - Prioritize reusability and maintainability when creating components - avoid overly complex or one-off implementations +## Deprecated Patterns to Avoid +- **Don't use Mixins** +- Ember mixins (`Mixin.create()`) are deprecated and being phased out +- Convert existing mixin functionality to utility modules or service injection instead +- Use utility functions or services for shared logic between classes +- **Code Review**: Flag any imports from `@ember/object/mixin` or `*.extend(SomeMixin)` patterns + +```javascript +// DEPRECATED: Don't use mixins +import Mixin from '@ember/object/mixin'; +import SomeMixin from 'vault/mixins/some-mixin'; +export default Route.extend(SomeMixin, { /* ... */ }); + +// Instead, use utility functions +import { utilityFunction } from 'vault/utils/utility-helpers'; +export default class MyRoute extends Route { + someMethod() { + return utilityFunction(this); + } +} +``` + ## Asynchronous Programming - Use `async`/`await` with proper error handling in `try`/`catch` blocks - Only use `@task` from ember-concurrency when you need specific features like cancellation or `task.isRunning` state management diff --git a/.github/instructions/generic/ember_tests.instructions.md b/.github/instructions/generic/ember_tests.instructions.md index 86a6bb525d..af2c9212a6 100644 --- a/.github/instructions/generic/ember_tests.instructions.md +++ b/.github/instructions/generic/ember_tests.instructions.md @@ -25,6 +25,47 @@ This document provides testing standards and best practices for HashiCorp Ember. # Testing Standards +## Test Focus and Purpose +- **Test repository-specific business logic**, not framework internals or external library functionality +- Focus on testing the behavior and logic that your application implements +- Avoid testing that framework methods exist or that imported functions are functions +- Test how your code interacts with frameworks, not the frameworks themselves + +**What TO test:** +- Business logic and application-specific behavior +- Component interactions and state changes +- Data transformations and computations +- User workflows and edge cases +- Error handling and validation logic + +**What NOT to test:** +- That Ember route methods like `modelFor()` or `paramsFor()` exist +- That imported utility functions are of type 'function' +- Framework-provided functionality (Ember, QUnit, etc.) +- Third-party library internals +- Basic JavaScript language features + +Example of unnecessary framework testing: +```javascript +// Don't do this - testing framework internals +test('route has standard methods', function (assert) { + assert.ok(this.route.modelFor, 'Route has modelFor method'); + assert.strictEqual(typeof myUtilFunction, 'function', 'function is a function'); +}); +``` + +Example of proper business logic testing: +```javascript +// Do this - test your application logic +test('getBackendEffectiveType returns correct type for external plugins', function (assert) { + this.route.modelFor = this.stub().returns({ engineType: 'vault-plugin-secrets-keymgmt' }); + + const effectiveType = getBackendEffectiveType(this.route); + + assert.strictEqual(effectiveType, 'keymgmt', 'External keymgmt plugin returns effective type'); +}); +``` + ## Test Quality - Use `assert.true()` or `assert.false()` instead of `assert.ok()` for boolean checks - Provide descriptive assertion messages that explain what is being verified