The Wayback Machine - https://web.archive.org/web/20200915181932/https://github.com/Nickersoft/push.js/pull/189
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

Update typings for default export and config #189

Merged
merged 1 commit into from Oct 20, 2018

Conversation

@natsukagami
Copy link
Contributor

natsukagami commented Oct 9, 2018

The current index.d.ts file, when compiled with TypeScript 3.1, would give the following error:

./node_modules/push.js/index.d.ts:3:20
    TS2714: The expression of an export assignment must be an identifier or qualified name in an ambient context.
./node_modules/push.js/index.d.ts:14:9
    TS7010: 'config', which lacks return-type annotation, implicitly has an 'any' return type.

I attempted a minimal fix to the above problem.


This change is Reviewable

The current index.d.ts file, when compiled with TypeScript 3.1, would give the following error:

```
./node_modules/push.js/index.d.ts:3:20
    TS2714: The expression of an export assignment must be an identifier or qualified name in an ambient context.
./node_modules/push.js/index.d.ts:14:9
    TS7010: 'config', which lacks return-type annotation, implicitly has an 'any' return type.
```
Copy link

VicXXX left a comment

Hey can we get that in in TypeScript 2.9 i get a lot of warnings because of that.

@natsukagami
Copy link
Contributor Author

natsukagami commented Oct 18, 2018

What do you mean? Compiling my patch with tsc 2.9.2 produces no warnings at all.

@VicXXX
Copy link

VicXXX commented Oct 18, 2018

What do you mean? Compiling my patch with tsc 2.9.2 produces no warnings at all.

I mean the current Push.js pulled from NPM has those errors. I want your PR to go in.

@theLufenk
Copy link
Collaborator

theLufenk commented Oct 20, 2018

@natsukagami Welcome to push.js and thanks a lot for your first PR !

The fix makes sense.
We already have a related issue in #191 .

I would try to merge and draft a release asap as it seems to be already affecting people.

@theLufenk theLufenk merged commit 8ebfdad into Nickersoft:master Oct 20, 2018
3 checks passed
3 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - package.json (Nickersoft) No manifest changes detected
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.