Skip to content

Export correct types for RequestHandlerContext.dataSource for plugins #6045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joshuali925 opened this issue Mar 5, 2024 · 4 comments · Fixed by #6108
Closed

Export correct types for RequestHandlerContext.dataSource for plugins #6045

joshuali925 opened this issue Mar 5, 2024 · 4 comments · Fixed by #6108
Assignees
Labels
enhancement New feature or request v2.13.0

Comments

@joshuali925
Copy link
Member

Is your feature request related to a problem? Please describe.

currently in plugins when we do context.dataSource in route handler, typescript complains Property 'dataSource' does not exist on type 'RequestHandlerContext'., and it's hard for developers to know the schema of available methods under it.

Describe the solution you'd like

Core to provide types correctly for RequestHandlerContext

Describe alternatives you've considered

I'm aware that plugins can add

declare module '../../../src/core/server' {
  interface RequestHandlerContext {
    dataSource: DataSourcePluginRequestContext;
  }
}

But this is unreliable and duplicate work for all plugins, especially because plugins have to also add DataSourcePluginRequestContext definition since it's not a top level export

export interface DataSourcePluginRequestContext {
  opensearch: {
    getClient: (dataSourceId: string) => Promise<OpenSearchClient>;
    legacy: {
      getClient: (
        dataSourceId: string
      ) => {
        callAPI: (
          endpoint: string,
          clientParams: Record<string, any>,
          options?: LegacyCallAPIOptions
        ) => Promise<unknown>;
      };
    };
  };
}

this can cause issues if upstream API changes and types become inconsistent. For something like dataSource that should be universal rather than plugin specific, can this be made available to plugins in core?

Additional context

somewhat related #4274

@seraphjiang
Copy link
Member

seraphjiang commented Mar 6, 2024

@joshuali925 are you looking for export that interface here?

export { DataSourcePluginSetup, DataSourcePluginStart } from './types';

@Flyingliuhub @BionIT would you confirm with @joshuali925 and work with team to help plugin teams

@joshuali925
Copy link
Member Author

@seraphjiang yes, or would be better to make dataSource available through core's RequestHandlerContext type so that

declare module '../../../src/core/server' {
  interface RequestHandlerContext {
    dataSource: DataSourcePluginRequestContext;
  }
}

doesn't need to be added and maintained manually by each plugin

@seraphjiang
Copy link
Member

@zhyuanqi would you help this

@BionIT
Copy link
Collaborator

BionIT commented Mar 12, 2024

Hi @joshuali925, since data source plugin is also a plugin which does not exist in src/core, core plugins can consume the merged declaration to consume data source context interface, while external plugins can not. We suggest each external plugin to use the following to consume the type, and in the PR, we have exported the DataSourcePluginRequestContext at top level and I saw that search relevance plugin uses the following as well to declare merge, and the change only involves adding data source with import from data source server path. Hope it makes sense.

export interface SearchRelevancePluginRequestContext {
  metricsService: MetricsServiceSetup;
}

declare module '../../../src/core/server' {
  interface RequestHandlerContext {
    searchRelevance: SearchRelevancePluginRequestContext;
    dataSource: DataSourcePluginRequestContext;
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2.13.0
Projects
None yet
4 participants