Skip to content

Check existence of navigator before using it#1475

Merged
twiss merged 2 commits intoopenpgpjs:mainfrom
twiss:check-navigator-existence
Feb 11, 2022
Merged

Check existence of navigator before using it#1475
twiss merged 2 commits intoopenpgpjs:mainfrom
twiss:check-navigator-existence

Conversation

@twiss
Copy link
Copy Markdown
Member

@twiss twiss commented Feb 1, 2022

Fix #1464. Depends on #1474.

Comment thread src/util.js Outdated
twiss and others added 2 commits February 10, 2022 21:36
Co-authored-by: larabr <larabr+github@protonmail.com>
@twiss twiss force-pushed the check-navigator-existence branch from cd26a42 to 3c421d4 Compare February 10, 2022 20:36
@twiss twiss merged commit f93f59e into openpgpjs:main Feb 11, 2022
@twiss twiss deleted the check-navigator-existence branch February 11, 2022 12:33
Comment thread src/util.js
}

return navigator.hardwareConcurrency || 1;
return (typeof navigator !== 'undefined' && navigator.hardwareConcurrency) || 1;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate you trying to fix this problem, but just so you know: it doesn't fix the Cloudfare issue because it's still accessing a prohibited API. The mere attempt to read navigator is what's not allowed.
I tried to explain that in my pull request.
I will try to check with cloudflare if they could allow code like this to "pass"... but I thought you would be interested in knowing that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand - but even after your PR, you still had to shim util.isEdge, afaiu - after this PR and #1474, you can shim util.getHardwareConcurrency instead :)

Other than that, please do check with Cloudflare, as this is the standard way to check for the existence of global objects, it shouldn't cause issues.

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.

Cannot deploy to Cloudflare Worker due to dependency on browser-only "navigator"

3 participants