-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Fix incorrect TypeScript typings #515
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your PR, however let's first analyze the issue of #514
types/index.d.ts
Outdated
@@ -6,7 +6,7 @@ | |||
// Copyright (c) Microsoft Open Technologies Inc | |||
// Licensed under the MIT license. | |||
// TypeScript Version: 2.3 | |||
type channel = "loadstart" | "loadstop" | "loaderror" | "exit" | "message"; | |||
type channel = "loadstart" | "loadstop" | "loaderror" | "exit" | "message" | string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this makes the type useless if we just use a string
. Let's continue the discussion in #514 where I've added a question.
Updated to an agreed solution in #514 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks!
* master: docs: replaces outdated transition and presentation style links (apache#662) chore: remove deprecated orientation methods (apache#666) Fix incorrect TypeScript typings (apache#515) [apacheGH-652] add check for openInSystem postNotification (apache#654) ci: updates Node.js versions (apache#659) chore(npm): improve ignore list (apache#658) fix(android): Reset lefttoright if not set (apache#442) [android] Correcting the documentation regarding lefttoright opt… (apache#648) (android) Added option to turn on/off fullscreen mode in Android (apache#634) Android apacheGH-470 InAppBrowser: java.lang.IllegalArgumentException (apache#616) breaking(ios): remove UIWebView (apache#635) chore(release): 3.2.1-dev
Platforms affected
TypeScript based Cordova projects.
Motivation and Context
Fixes #514.
Description
There was a regression in 3.1.0 since 3.0.0, whereby typings did not allow for an arbitrary
string
type for event listeners. In case your TypeScript project contains anything else extendingWindow
, it would cause the build to fail.Testing
Ran official 3.1.0 from apache, build failures as described in linked issue.
Ran this fork, build succeeds.
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)