Skip to content
This repository has been archived by the owner on Jun 28, 2021. It is now read-only.

[Fixes #10] Fix build errors #14

Merged
merged 4 commits into from
Dec 30, 2016
Merged

Conversation

bluepichu
Copy link
Collaborator

This PR fixes some TS errors that were in the previous version and switches from @types/pixi.js to pixi-typescript for the Pixi typings definition files, as the latter is generally kept more up-to-date. At some point we should hopefully be able to revert to @types/pixi.js or remove the dependency altogether depending on how the folks at pixi-typescript decide to handle definition management long-term.

@bluepichu
Copy link
Collaborator Author

Yikes, this isn't ready. I didn't realize that this totally breaks the .d.ts output and makes it unusable.

@bluepichu
Copy link
Collaborator Author

Unfortunately, I fear there's no way to fix the .d.ts until Pixi 4.3.0 definitions hit DefinitelyTyped (which have failed twice already, see DefinitelyTyped/DefinitelyTyped#13505 and DefinitelyTyped/DefinitelyTyped#13515). Thus, while this is sufficient to fix #10 and allows the package to be used client-side (e.g. via a <script> tag to import it), it won't be usable via TS until after DT gets updated.

@tleunen
Copy link
Owner

tleunen commented Dec 27, 2016

So are we waiting for the updated types in @DefinitelyTyped before we can fix the issue here? Or would this be ok to merge, without full TS support, and then fix it once and for all when they got it fixed on their side?

It seems this also has the fix for #11, can we move this in another PR so it's easier to follow what's needed for one fix and the other?

@bluepichu
Copy link
Collaborator Author

It's probably best to merge this as-is (assuming this correctly fixes the build errors) and fix TS support once DT gets updated.

This doesn't have the fix for #11; that's in 63b946e, which is in the iss11 branch in my fork (which branches off of build-bugfixes after these commits, and thus depends on this PR).

@bluepichu
Copy link
Collaborator Author

bluepichu commented Dec 28, 2016

Counterpoint: we can reject this PR and hope that DefinitelyTyped/DefinitelyTyped#13579 goes through. It looks like it's passed their PR checks so hopefully that should get pulled in the next day or so, which should make it easier to do some cleanup before merging fixes to #10 and #11 (and hopefully #12 not too long thereafter).

@clark-stevenson
Copy link

clark-stevenson commented Dec 29, 2016

Andy managed to merge v4.3.0 to DT after my failed attempts!

@bluepichu
Copy link
Collaborator Author

Thanks @clark-stevenson! Fixing the types now so we can get this merged in.

@bluepichu
Copy link
Collaborator Author

...more accurately, fixing this as soon as the DT bot updates the NPM module. 😃

@bluepichu
Copy link
Collaborator Author

Welp, scratch that. pixijs/pixi-typescript#133 never got merged in with the DT update so now we need to wait for that to get merged in as well.

@bluepichu
Copy link
Collaborator Author

Now blocked on DefinitelyTyped/DefinitelyTyped#13631.

@bluepichu
Copy link
Collaborator Author

I think this is finally ready!

@tleunen tleunen merged commit 4fee646 into tleunen:master Dec 30, 2016
@bluepichu bluepichu deleted the build-bugfixes branch January 2, 2017 04:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants