Skip to content

fix: correct the spread order for http_emitter options #225

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
wants to merge 2 commits into from

Conversation

lholmquist
Copy link
Contributor

fixes: #223

I would like to add a test for this, but I think some type definitions need to be added so typescript doesn't complain about me trying to pass in more options in the send method. Keeping as Draft for now

Signed-off-by: Lucas Holmquist [email protected]

@lholmquist
Copy link
Contributor Author

While this should be a fairly straightforward change, i'm running into issues related to Typescript when adding in the tests.

I know basic's, but I'm not sure how to proceed when trying to declare the type for a variable object that uses the spread operator.

@grant perhaps you have some insight, since from my reading of other issues, you were the most vocal for adding typescript support. Thanks in advance

Also cc @lance

@grant
Copy link
Member

grant commented Jun 15, 2020

That's a bad bug. Thanks for the PR!

I too have seen many TypeScript issues due to the fact this repo is half js, half ts, which causes issues with types due to inferred types.

I've added an issue here: #203
I'll talk about that issue there.

@lance
Copy link
Member

lance commented Jun 18, 2020

I don't know. It could be a while before the rewrite lands and it might be that @c-pius may want to ref master for the time being? Care to chime in @c-pius?

@c-pius
Copy link

c-pius commented Jun 22, 2020

From my side there is no urgency. Anyway, thanks for your great work with CE and this SDK!

@lholmquist
Copy link
Contributor Author

closing this since it will be fixed in #226

@lholmquist lholmquist closed this Jun 26, 2020
@lholmquist lholmquist deleted the 223-spread-order branch June 26, 2020 01:42
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

Successfully merging this pull request may close these issues.

Passing custom headers through to http.request()
4 participants