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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

typing error on requestParam.d.ts #1924

Closed
potreco opened this issue Jun 28, 2023 · 10 comments
Closed

typing error on requestParam.d.ts #1924

potreco opened this issue Jun 28, 2023 · 10 comments
Assignees

Comments

@potreco
Copy link

potreco commented Jun 28, 2023

馃悰 Bug Report

A type error on SnapshotGet, on file requestParam.d.ts file.
I'm opened a PR to fix this: #1925

MicrosoftTeams-image

Lines: 2632 and 2636, change both to number.

To Reproduce

Steps to reproduce the behavior:

  • Install this lib on a NestJS project
  • Build the project

Expected behavior

Build occur normally

Your Environment

  • node version: 16
  • @elastic/elasticsearch version: 7.17.11
  • os: Mac, Windows, Linux
@adambiggs
Copy link

Is type checking part of the CI pipeline?

@MichaelRoskin
Copy link

MichaelRoskin commented Jun 29, 2023

Im getting the same...
kids, that's what happens when you npm install without a package-lock.json - lesson learned

@distracteddev
Copy link

Any chance of an update on when this release can be rolled back or a forward-patch made?

@JoshMock JoshMock self-assigned this Jun 29, 2023
@JoshMock JoshMock added the bug label Jun 29, 2023
@JoshMock
Copy link
Member

JoshMock commented Jun 29, 2023

Thanks for the report, all. The issue likely will need to be resolved in the underlying code generator or the Elasticsearch specification that the generator uses. Once it's applied it will be published in the next 7.x release.

In the meantime, you can pin your dependencies to 7.17.0.

There is a type-check done before publishing, but it appears to have failed silently or been bypassed somehow. I'll investigate that separately to ensure failures successfully block publishes going forward.

JoshMock added a commit that referenced this issue Jun 29, 2023
These prevented the TypeScript type-checker from running prior to a
release, and skipping (or, ideally, fixing if time were on our side)
the tests would have revealed a problem in
#1924 prior to
7.17.11 being published.
@JoshMock
Copy link
Member

The fix in #1927 will resolve this issue. Unfortunately the npm package did not qualify for unpublishing, so we will be publishing version 7.17.11-patch.1 shortly.

Once that publishes, you can either pin to version 7.17.11-patch.1 or 7.17.0 until we release 7.17.12. Versioning of the client is tied to the Elasticsearch release schedule, and we currently do not have a date for when that might release.

Thanks again for your patience, everyone. 馃檹

@JoshMock
Copy link
Member

7.17.11-patch.1 has been published to npm. Release notes around when to use the patch version are clarified on the release page.

@distracteddev
Copy link

distracteddev commented Jul 12, 2023

@JoshMock Just wanted to share my experience that this is not a complete solution because many dependencies are included by packages we do not have control over, so there is no good way to pin the package to that version across all dependents, across all repos. Although mistakes are definitely understandable, it seems there is a gap in the release policies / use of semver if you are unable to release a patch to fix a broken version.

@JoshMock
Copy link
Member

Thanks for the feedback @distracteddev. Just this week, we on the Elasticsearch clients team have decided that we'll detach patch releases of the clients from the Elasticsearch stack release schedule, in large part so that we can release client-specific patches like this one in a more timely manner without forcing users to break out of semver to get critical fixes. 馃帀

Given that, I'm going to release 7.11.12 soon (hopefully before the end of this week), which will be functionally equivalent to 7.17.11-patch.1. Since the version number will be a proper semver patch release, npm should pick up automatically even if this client is a sub-dependency of another dependency. Hopefully that helps!

cc @technige

@JoshMock JoshMock reopened this Jul 12, 2023
@distracteddev
Copy link

Wow, what an incredible response! Thanks a ton.

@JoshMock
Copy link
Member

7.17.12 is now up on npm. Glad we could circle back for an extra win on this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants