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

Example of broken types #692

Closed
wants to merge 2 commits into from
Closed

Example of broken types #692

wants to merge 2 commits into from

Conversation

jperl
Copy link
Contributor

@jperl jperl commented Jan 28, 2020

This PR is not intended to be merged, just an example of a top level export issue and types issue with playwright-core@0.9.23 and typescript.

If you try to run npm run build in the playwright-test-types package you will see the types error

Screen Shot 2020-01-27 at 6 02 31 PM

I think it would be useful to have a test that playwright can be imported by a typescript package and built, and to test top level exports like playwright.devices work.

I would be happy to add this test, if you could point me in the right direction of how you would like it to work with the test runner.

jperl added a commit to qawolf/qawolf that referenced this pull request Jan 28, 2020
@jperl jperl changed the title fix playwright.devices["iPhone 7"] Example of broken types Jan 28, 2020
@jperl jperl closed this Jan 28, 2020
@jperl
Copy link
Contributor Author

jperl commented Jan 28, 2020

Alternatively if there were tests written with typescript they would also catch this kind of issue.

@aslushnikov
Copy link
Collaborator

ah, that's a great finding! Thanks a lot!

Addressed this in #698
Will publish 0.9.24 rn to fix this

@aslushnikov
Copy link
Collaborator

Oh wait, I checked your code first and saw the wrong export that we did for device descriptors - this is fixed now.

However, this is something more involved than I initially thought. 🧐cc @JoelEinbinder

@JoelEinbinder JoelEinbinder self-assigned this Jan 28, 2020
@jperl
Copy link
Contributor Author

jperl commented Jan 28, 2020

Thanks for the quick response. At first I was going to submit a PR with all the fixes, but I kept running into other types issues and ran out of time before a gym class.

This PR resolves the build issue #699

However I think having the tests in typescript would prevent future whack-a-mole types issues like this.

@jperl jperl mentioned this pull request Jan 28, 2020
aslushnikov pushed a commit that referenced this pull request Jan 28, 2020
@jperl jperl deleted the fix-device-export branch January 29, 2020 17:44
@jperl jperl mentioned this pull request Jan 29, 2020
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.

3 participants