search

karaggeorge / mac-screen-capture-permissions

Check and request permission to capture the screen
117 stars 35 forks source link

Simplify code #8

Closed saghul closed 3 years ago

saghul commented 3 years ago

Use runtime availability detection, instead of having 2 bindings.

saghul commented 3 years ago

Updated!

csett86 commented 3 years ago

@saghul unfortunately it does not work as desired: I tried this locally on macOS 11 and it always uses the "legacy" code path with CGDisplayStreamCreateWithDispatchQueue, even when running on macOS 11. This results in the old behaviour to always prompt the user to allow screensharing, even if only checking for the permission.

csett86 commented 3 years ago

Only when building it with "MACOSX_DEPLOYMENT_TARGET": "11.0", I get the new API. But my doubt is that this will then still run on 10.15 (but cannot verify as I don't have a test device for that)

saghul commented 3 years ago

Hum that's weird! I'll take another look the upcoming week.

saghul commented 3 years ago

I couldn't reproduce your results.

I just built the module on macOS 10.15 and ran the example, uses the old API. I then rsync-ed the binary to a 11.0 mac and ran it again. It uses the new API.

I added some logs to make sure the right branch was taken.

csett86 commented 3 years ago

@saghul I also can no longer reproduce the issue, so ignore my previous remarks. Thank you for the solution!

karaggeorge commented 3 years ago

Thank you both for all the work! Sorry I've been so slow with PRs/issues. I will merge this, but I'm not on macos 11 so I can't build this locally to publish. I'll be cloning this on a macos 11 machine later this week to test/publish

csett86 commented 3 years ago

@karaggeorge There is no need to build it to cut a release and publish it. Since #6 the binaries are build locally during install and are no longer shipped with the package.

As a side node: This also solves issue #5 as the binaries are then built and signed locally correctly. So as soon as you cut a release, #5 can also be closed.

In addition, if you want to build it for testing before publishing, you only need an macOS 11 SDK, not macOS 11 itself. I also build the module this way on github actions, as macOS 11 is not available there either: https://github.com/csett86/mac-screen-capture-permissions/blob/master/.github/workflows/node.js.yml#L20

I can upstream the github action build as well if you like.

To move this forward, I locally tested current master (b65758c35d9b784bff2019b2c4a62d2c703d07a6) and the module works as expected.

csett86 commented 3 years ago

@karaggeorge I have tested current master (b65758c) and its working fine. Anything else I can help to get a release cut from current master and published on npm?

karaggeorge commented 3 years ago

Hey, @csett86 I will actually be releasing it under a new major version today. Sorry for the delay

karaggeorge commented 3 years ago

Released as v2.0.0

csett86 commented 3 years ago

Thanks a lot @karaggeorge!