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

add WebGL 1.0 types #2764

Closed
wants to merge 3 commits into from
Closed

add WebGL 1.0 types #2764

wants to merge 3 commits into from

Conversation

@gre
Copy link
Contributor

gre commented Nov 5, 2016

@gre
Copy link
Contributor Author

gre commented Nov 6, 2016

there is one unperfect thing in my current definition:

WebGLRenderingContext.LINEAR

does pass (it's a static field in the spec).

but

gl.LINEAR

is valid and widely used code but won't work (property not found) because it's a static not part of the instance.

Should I duplicate the field in both the class and the instance, or is there a smart way?

BTW, this is exactly the same on Node :

https://flowtype.org/try/#0G4QwTgBAJgvFD2BjArgWwKYDsAuA6RY6I26AogDboY4AUARFAJbB0CUAUFLqQDKkCypAHIAVAPpCA8gBFSnfPEwBnbGGSJs8MNz6DREmaSA

both should be valid

@gre
Copy link
Contributor Author

gre commented Nov 6, 2016

i've duplicated the webgl statics into fields. seems to work fine.

@gre
Copy link
Contributor Author

gre commented Nov 14, 2016

Can you guys take a look at accepting this PR ?
I can't wait to have this shipped in flow for bunch of GL libs I have.

Thanks

@facebook-github-bot
Copy link

facebook-github-bot commented Nov 18, 2016

@thejameskyle has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@jamiebuilds
Copy link
Contributor

jamiebuilds commented Nov 18, 2016

You have a number of failing tests: https://travis-ci.org/facebook/flow/jobs/173626828#L724

@gre
Copy link
Contributor Author

gre commented Nov 18, 2016

as far as I understand, the failing tests are just line number changes right?

It's all about file number changes in dom.js, because I added lines in that file.

example

-    function type. Callable signature not found in. See lib: dom.js:1906
+    function type. Callable signature not found in. See lib: dom.js:2789
@jamiebuilds
Copy link
Contributor

jamiebuilds commented Nov 18, 2016

I'll do it on my end, but to re-record those you just need to do:

# first: https://github.com/facebook/flow/tree/master#building-flow
make build
./runtests.sh bin/flow    # check that everything is just line number changes
./runtests.sh -r bin/flow # re-record tests
./runtests.sh bin/flow    # check it again to make sure everything worked.
@jamiebuilds
Copy link
Contributor

jamiebuilds commented Nov 18, 2016

Oh you did it. Cool, but there's now a conflict. Can you rebase and retry?

@mroch
Copy link
Contributor

mroch commented Nov 18, 2016

I typically just do ./runtests.sh -r bin/flow and git diff to make sure it only made line number changes. -r should be pretty reliable, and travis would catch it if something went horribly wrong :)

gre added 2 commits Nov 18, 2016
@gre gre force-pushed the gre:webgl1 branch from 37df7f2 to 7c7b4ac Nov 18, 2016
@gre
Copy link
Contributor Author

gre commented Nov 18, 2016

I've rebased my branch, but for some reason, the test are still pending, mmh

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

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.