[internal] Change how we initialise plugins #151
Conversation
yoannmoinet
requested review from
nchapma2 and
elbywan
and removed request for
a team and
nchapma2
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the plugin initialization by shifting the enabling logic into individual product plugins and simplifying the factory’s initialization. Key changes include updating type signatures for plugin functions, removing legacy conditional checks from the factory and integrity commands, and adding tests to verify that plugins only initialize when explicitly enabled.
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/tools/src/helpers.ts | Updates the generic use for getSupportedBundlers. |
| packages/tools/src/commands/integrity/files.ts | Removes test path checks from the CODEOWNERS update logic. |
| packages/tools/src/commands/create-plugin/templates.ts | Adjusts import types and plugin option validation logic. |
| packages/tools/src/commands/create-plugin/index.ts | Removes test path insertion in the CODEOWNERS file. |
| packages/tools/src/commands/create-plugin/index.test.ts | Updates tests to match the new CODEOWNERS file content. |
| packages/tests/src/_jest/helpers/mocks.ts | Revises the logger mock implementation. |
| packages/plugins/telemetry/* | Updates function signature and adds early returns when disabled. |
| packages/plugins/rum/* | Updates type definitions and configuration validation. |
| packages/plugins/error-tracking/* | Changes plugin option types and adds early returns when disabled. |
| packages/factory/src/* | Refactors the plugin factory to iterate through product plugins. |
| packages/core/src/types.ts | Removes the generic type for GetPlugins. |
Files not reviewed (1)
- .github/CODEOWNERS: Language not supported
Comments suppressed due to low confidence (1)
packages/tests/src/_jest/helpers/mocks.ts:145
- The variable 'mockLogger' is referenced but not defined in this file. Consider defining or importing 'mockLogger' to ensure the logger mock works as expected.
getLogger: jest.fn(() => mockLogger),
.github/CODEOWNERS
Outdated
There was a problem hiding this comment.
This was forgotten from when I moved test files closer to what they test.
global.d.ts
Outdated
There was a problem hiding this comment.
Just alpha sort of the keys.
packages/factory/src/index.test.ts
Outdated
There was a problem hiding this comment.
The responsibility has moved, no need to test for this in the factory anymore.
There was a problem hiding this comment.
Missed from the test files move's PR.
There was a problem hiding this comment.
Missed from the test files move's PR.
| const testsPath = `packages/tests/src/plugins/${plugin.slug}`; | ||
| const pluginPath = `${plugin.location}`; | ||
|
|
||
| if (!codeowners.includes(testsPath)) { | ||
| errors.push( | ||
| `[${error}] Missing ${title}'s tests (${dim(testsPath)}) in ${green(codeownersPath)}.`, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missed from the test files move's PR.
What and why?
In preparation of [feat] Add synthetics plugin we need to offload the enabling capability onto the plugins instead of on the factory.
So the plugins have more control over when to be enabled or not instead of only relying on the
disabled: trueconfiguration.How?