Skip to content
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

[BUG] opensearchtypes export and api/types subpath have been removed from v3, cannot import API inner types anymore ? #955

Closed
xfournet opened this issue Jan 8, 2025 · 11 comments · Fixed by #957
Labels
🧗 enhancement New feature or request

Comments

@xfournet
Copy link
Contributor

xfournet commented Jan 8, 2025

What is the bug?

In v2 it was possible to access API inner types (QueryDslQueryContainer as an example) directly, either using opensearchtypes export from main index, or since #674 using the api/types subpath

eg with opensearchtypes

import type { opensearchtypes } from '@opensearch-project/opensearch';

const buildQuery = (): opensearchtypes.QueryDslQueryContainer => {
  return {
    // TODO
  };
};

and with api/types subpath

import type { QueryDslQueryContainer } from '@opensearch-project/opensearch/api/types';

const buildQuery = (): QueryDslQueryContainer => {
  return {
    // TODO
  };
};

How can one reproduce the bug?

After upgrade to v3, opensearchtypes export is no more present and api/types expotred subpath has been removed from package.json
(BTW, I did see mention of these breaking changes in the release note, so i may suspect theses changes are not intentional ?)

The corresponding type in v3 is QueryContainer in api/_types/_common.query_dsl.d.ts but it's not exported by the module

I have tried

import type { QueryContainer } from '@opensearch-project/opensearch/api/_types/_common.query_dsl';

const buildQuery = (): QueryContainer => {
  return {
    // TODO
  };
};

it's recognized in the IDE, but, as expected, rejected by typescript

test.ts:1:37 - error TS2307: Cannot find module '@opensearch-project/opensearch/api/_types/_common.query_dsl' or its corresponding type declarations.

1 import type { QueryContainer } from '@opensearch-project/opensearch/api/_types/_common.query_dsl';

I may have miss something, but so far I didn't found a way to import this type.

What is the expected behavior?

Having a simple way to import API inner types directly

What is your host/environment?

Non relevant

Do you have any screenshots?

Non relevant

Do you have any additional context?

No

@nhtruong
Copy link
Collaborator

nhtruong commented Jan 8, 2025

Thanks for reporting the issue. The change to how type definitions are accessed is a breaking change from 2.X to 3.X as the types from 3.X are generated from the spec instead of hand written. Check out this upgrading guide on typescript.

There 2 ways to access the QueryContainer type. One is directly referencing the location of the type definition like you did, the other is through the the request/response types exposed by the API module. Both work for me:

// playground.ts
import { type API } from '@opensearch-project/opensearch';
import type { QueryContainer } from '@opensearch-project/opensearch/api/_types/_common.query_dsl';

const buildQuery = (): QueryContainer => {
  return {
    wildcard: {}
  };
};

const query: NonNullable<API.UpdateByQuery_Request['body']>['query'] = {
  span_within: {
    big: {},
    little: {}
  }
}

console.log(buildQuery());
console.log(query);
// > ts-node playground.ts
{ wildcard: {} }
{ span_within: { big: {}, little: {} } }

Regarding accessing the component type via the API module

@xfournet
Copy link
Contributor Author

xfournet commented Jan 8, 2025

As mentioned in my initial comment, tsc throws an error when trying to import @opensearch-project/opensearch/api/_types/_common.query_dsl, as this module isn’t exported in the package.json
image

Regarding the second proposal, it’s fragile and much less readable compared to v2, where types were directly accessible. QueryContainer is a simple case, and the workaround is already quite messy ;) For other types like ErrorResponseBase, FieldValue, SortResult, SortOrder, retrieving them becomes even trickier.

It seems to me that the removal of the opensearchtypes and types/api exports was overlooked in v3 since as there is no practicable workaround to compensate that. The export of inner API types is really crucial for advanced or dynamic query creation.

Would be possible to reintroduce this export ?

@nhtruong
Copy link
Collaborator

nhtruong commented Jan 8, 2025

We can export api/_types in package.json so that you can access this component type with import type { QueryContainer } from '@opensearch-project/opensearch/api/_types/_common.query_dsl';

opensearchtypes were handwritten and replacing it with generated types was intentional. This is an expected breaking change and the main reason we did a major version bump.

We can update the generator to introduce import type { ComponentTypes } from '@opensearch-project/opensearch' that aggregates all types defined in api/_types and let you access QueryContainer with ComponentTypes.Common_QueryDsl_QueryContainer if you think it's easier to use but this will take some time to realize.

cost query: NonNullable<API.UpdateByQuery_Request['body']>['query'] is not pretty but it has the advantage of indicating the intention of declaring const query to be used in a UpdateByQuery request (it's unfortunate that body is optional, hence NonNullable). I see that it's a matter of opinion, and we would add a direct way to access this type. Though do note that the names of these component types might change when the spec is updated (even though this rarely happens).

@xfournet
Copy link
Contributor Author

xfournet commented Jan 8, 2025

Yes, having the auto-generated types directly accessible via a ComponentTypes export would be ideal.

I understand your perspective regarding accessing types through the API root. However, some types are shared across different APIs. For instance, QueryContainer is referenced in DeleteByQuery_RequestBody, UpdateByQuery_RequestBody, Search_RequestBody, etc. A method like buildQuery(): QueryContainer can be used across all these APIs. Conversely, defining it as buildQuery(): NonNullable<UpdateByQuery_Request['query']> feels awkward when the same method is used for DeleteByQuery or Search.

@dblock
Copy link
Member

dblock commented Jan 8, 2025

@xfournet want to try a PR along the lines of what you're proposing?

@dblock dblock added the 🧗 enhancement New feature or request label Jan 8, 2025
@xfournet
Copy link
Contributor Author

xfournet commented Jan 8, 2025

Indeed I was trying to figure out how to add that in the API generator. First I tried to execute the API generator, but no luck so far i get an error. Is there a precise version of the opensearch-openapi.yaml to use ? I get errors with latest and v0.1.0

❯ npm run generate_api

> [email protected] generate_api
> ts-node src/generate_api.ts

D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesContainer.ts:67
    if (!container) throw new Error(`Container not found for: ${ref} -> ${file_path}`)
                          ^
Error: Container not found for: #/components/schemas/_common:Host -> _types/_common:Host.d.ts
    at ComponentTypesContainer.ref_to_container (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesContainer.ts:67:27)
    at ComponentTypesContainer.ref_to_obj (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesContainer.ts:52:28)
    at TypesFileRenderder.#render_schema (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:52:53)
    at D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:112:49
    at Array.map (<anonymous>)
    at TypesFileRenderder.#render_simple_obj (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:110:8)
    at TypesFileRenderder.#render_schema (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:66:35)
    at D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:37:47
    at Array.map (<anonymous>)
    at TypesFileRenderder.view (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:36:37)

@nhtruong
Copy link
Collaborator

nhtruong commented Jan 8, 2025

You should download the spec first.

The quickest fix is adding api/_types folder to the export.

If you opt to create an OpenSearchTypes module, we already generate a very similar module called API that aggregates all request and response types that you can model after. Here's the rendering logic and template.

@xfournet
Copy link
Contributor Author

xfournet commented Jan 8, 2025

I already downloaded the spec. But I had this error during the API generation. I increased the stacktrace limit to have more information if it can help ?

❯ npm run download_spec

> [email protected] download_spec
> curl -L -X GET "https://github.com/opensearch-project/opensearch-api-specification/releases/download/main-latest/opensearch-openapi.yaml" -o opensearch-openapi.yaml

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 2312k  100 2312k    0     0  3972k      0 --:--:-- --:--:-- --:--:-- 30.7M
 D:sourcesgithubopensearch-jsapi_generator  main                                                                                                        ﮫ 909ms  00:51:50
❯ npm run generate_api

> [email protected] generate_api
> ts-node src/generate_api.ts

D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesContainer.ts:67
    if (!container) throw new Error(`Container not found for: ${ref} -> ${file_path}`)
                          ^
Error: Container not found for: #/components/schemas/_common___Host -> _types/_common.d.ts
    at ComponentTypesContainer.ref_to_container (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesContainer.ts:67:27)
    at ComponentTypesContainer.ref_to_obj (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesContainer.ts:52:28)
    at TypesFileRenderder.#render_schema (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:52:53)
    at D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:112:49
    at Array.map (<anonymous>)
    at TypesFileRenderder.#render_simple_obj (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:110:8)
    at TypesFileRenderder.#render_schema (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:66:35)
    at D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:37:47
    at Array.map (<anonymous>)
    at TypesFileRenderder.view (D:\sources\github\opensearch-js\api_generator\src\renderers\render_types\TypesFileRenderder.ts:36:37)
    at TypesFileRenderder.render (D:\sources\github\opensearch-js\api_generator\src\renderers\BaseRenderer.ts:28:25)
    at Generator.#generate_types_containers (D:\sources\github\opensearch-js\api_generator\src\Generator.ts:111:44)
    at Generator.generate (D:\sources\github\opensearch-js\api_generator\src\Generator.ts:57:36)
    at Object.<anonymous> (D:\sources\github\opensearch-js\api_generator\src\generate_api.ts:14:11)
    at Module._compile (node:internal/modules/cjs/loader:1740:14)
    at Module.m._compile (D:\sources\github\opensearch-js\api_generator\node_modules\ts-node\src\index.ts:1618:23)
    at node:internal/modules/cjs/loader:1905:10
    at Object.require.extensions.<computed> [as .ts] (D:\sources\github\opensearch-js\api_generator\node_modules\ts-node\src\index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1474:32)
    at Function._load (node:internal/modules/cjs/loader:1286:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:234:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:151:5)
    at phase4 (D:\sources\github\opensearch-js\api_generator\node_modules\ts-node\src\bin.ts:649:14)
    at bootstrap (D:\sources\github\opensearch-js\api_generator\node_modules\ts-node\src\bin.ts:95:10)
    at main (D:\sources\github\opensearch-js\api_generator\node_modules\ts-node\src\bin.ts:55:10)
    at Object.<anonymous> (D:\sources\github\opensearch-js\api_generator\node_modules\ts-node\src\bin.ts:800:3)
    at Module._compile (node:internal/modules/cjs/loader:1740:14)
    at Object.<anonymous> (node:internal/modules/cjs/loader:1905:10)
    at Module.load (node:internal/modules/cjs/loader:1474:32)
    at Function._load (node:internal/modules/cjs/loader:1286:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:234:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:151:5)
    at node:internal/main/run_main_module:33:47

@xfournet
Copy link
Contributor Author

xfournet commented Jan 9, 2025

Got it (Windows support) i will provide the fix for that

@xfournet xfournet mentioned this issue Jan 9, 2025
5 tasks
xfournet added a commit to xfournet/opensearch-js that referenced this issue Jan 9, 2025
@xfournet
Copy link
Contributor Author

xfournet commented Jan 9, 2025

See PR #957

xfournet added a commit to xfournet/opensearch-js that referenced this issue Jan 10, 2025
xfournet added a commit to xfournet/opensearch-js that referenced this issue Jan 10, 2025
nhtruong pushed a commit that referenced this issue Jan 10, 2025
Generate api/_types/index.d.ts (#955)

Signed-off-by: Xavier Fournet <[email protected]>
@nhtruong
Copy link
Collaborator

3.1.0 has been released.

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

Successfully merging a pull request may close this issue.

3 participants