fix: use getAsFileSystemHandle only in secure ctx

[rolandjitsu]

What kind of change does this PR introduce?

• Bug Fix
• Feature
• Refactoring
• Style
• Build
• Chore
• Documentation
• CI

Did you add tests for your changes?

• Yes, my code is well tested
• Not relevant

If relevant, did you update the documentation?

• Yes, I've updated the documentation
• Not relevant

Summary

Potential fix for react-dropzone/react-dropzone#1397.

Does this PR introduce a breaking change?

No.

Other information

[coveralls]

Pull Request Test Coverage Report for Build 12009643702

Details

• 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 11642347893:	0.0%
Covered Lines:	92
Relevant Lines:	92

---

💛 - Coveralls

[jonkoops]

Interesting, so only when in a secure context does getAsFileSystemHandle() return the expected values? What a strange issue, is there a bug/tracker for this behavior in the affected browsers?

[jonkoops]

Found the tracker here: https://issues.chromium.org/issues/40186242. I'll see if I can reproduce the original issue, as this really feels like something that should be solved by the Chromium team.

[rolandjitsu]

Interesting, so only when in a secure context does getAsFileSystemHandle() return the expected values? What a strange issue, is there a bug/tracker for this behavior in the affected browsers?

I think we can consider it an undefined behaviour when not in a secure context. I'd expect it to throw an exception when trying to use it, and not crash the browser.

[rolandjitsu]

Found the tracker here: https://issues.chromium.org/issues/40186242. I'll see if I can reproduce the original issue, as this really feels like something that should be solved by the Chromium team.

In any case, guarding for a secure context should still be applicable.

Wow, that issue is from 2021 😮

[jonkoops]

I think we can consider it an undefined behaviour when not in a secure context. I'd expect it to throw an exception when trying to use it, and not crash the browser.

I am really surprised it is even present. All other APIs that are behind a secure context are just straight up undefined, for example the Crypto API.

In any case, guarding for a secure context should still be applicable.

I agree, if this helps users now then we should land this. We can always remove it in a future version if the Chromium team ever decides to fix it.

Wow, that issue is from 2021 😮

And yes, that issue is really old, I am surprised it has slipped through the cracks.

[jonkoops]

LGTM!

[github-actions]

🎉 This PR is included in version 2.1.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[jonkoops]

@rolandjitsu in terms of backporting (fowardporting), shall I cherry-pick this onto the beta branch? Or do we handle this when we eventually merge beta into master?

[rolandjitsu]

@rolandjitsu in terms of backporting (fowardporting), shall I cherry-pick this onto the beta branch? Or do we handle this when we eventually merge beta into master?

I usually make sure beta is rebased onto master ... but I know semantic-release will hate that 😆

[jonkoops]

Yeah, that would be my approach as well if it wasn't for semantic-release. It has actually been a while since I've done anything merge based.

Alternative approach, we can cherry pick this fix on top of beta, and when we do a release rename the master branch to 2.x, and the beta branch to master (or main). WDYT?

[rolandjitsu]

Alternative approach, we can cherry pick this fix on top of beta, and when we do a release rename the master branch to 2.x, and the beta branch to master (or main). WDYT?

If it weren't for NPM, I'd usually remove all tags/releases, rebase and force push, then the rc.1 would be recreated (not ideal, but it keeps the commit history clean).

But with NPM in play, we must make sure semantic-release moves fwd with versioning:

1. Cherry pick would make increment the current rc ✅
2. master -> 2.x => semantic-release should do nothing; but I think that we need a major release before we make a maintenance branch as far as I recall (so a v3 needs to happen before we branch out into a maintenance branch); it's possible that we can make a 2.1.x which may work
3. beta -> master; this is uncharted territory; semantic-releases uses git tags and notes to figure out what todo (from what I understand), and renaming may fuck up things the same way as rebasing since the shas are not the same as it expects in master

We may need to use a merge with a commit to avoid the headaches; see https://semantic-release.gitbook.io/semantic-release/usage/workflow-configuration#merging-into-a-pre-release-branch.

[jonkoops]

2. master -> 2.x => semantic-release should do nothing; but I think that we need a major release before we make a maintenance branch as far as I recall (so a v3 needs to happen before we branch out into a maintenance branch); it's possible that we can make a 2.1.x which may work

This is manageable, we don't intend to do any releases when we rename master to 2.x until after we release v3, and then we would only do further 2.x releases for critical bug fixes (if we want to continue supporting 2.x), leaving it mostly stale.

3. beta -> master; this is uncharted territory; semantic-releases uses git tags and notes to figure out what todo (from what I understand), and renaming may fuck up things the same way as rebasing since the shas are not the same as it expects in master

This would not change the tags, they would be preserved and remain on the 2.x branch. So when semantic-releases does a index it should find all the information it needs.

[jonkoops]

I've created a backport for this PR to the beta branch under #122

[rolandjitsu]

2. master -> 2.x => semantic-release should do nothing; but I think that we need a major release before we make a maintenance branch as far as I recall (so a v3 needs to happen before we branch out into a maintenance branch); it's possible that we can make a 2.1.x which may work

This is manageable, we don't intend to do any releases when we rename master to 2.x until after we release v3, and then we would only do further 2.x releases for critical bug fixes (if we want to continue supporting 2.x), leaving it mostly stale.

3. beta -> master; this is uncharted territory; semantic-releases uses git tags and notes to figure out what todo (from what I understand), and renaming may fuck up things the same way as rebasing since the shas are not the same as it expects in master

This would not change the tags, they would be preserved and remain on the 2.x branch. So when semantic-releases does a index it should find all the information it needs.

I have a faint recollection of semantic-release not liking a maintenance branch for the current major before releasing a major. But it may have been a different scenario or workflow.

I was looking at https://semantic-release.gitbook.io/semantic-release/usage/workflow-configuration#maintenance-branches and it seems ok as long as we stay within the range enforced by semver (in 2.x we can only release < 2.1 - the current release in master).

I can go ahead and create the 2.x branch. It should release on the 2.1 channel.

[rolandjitsu]

Oh boy, our CI release file does not allow for 2.x (currently "[0-9]+.[0-9]+.x"), only 2.1.x.

2.1.x works as expected: https://github.com/react-dropzone/file-selector/actions/runs/12068422043/job/33653498193.

We could leave it at that.

[jonkoops]

I think we can hold of on branching to 2.x until we feel like the beta branch is getting close to be in a good enough state for a v3 release. There is no urgent need for doing a release at this time (although I do want to keep making steady progress), and we'll have to see how to tie this into the larger ecosystem (e.g. react-dropzone itself).

Probably 2.1.x is good enough for us now, I don't think we'll be landing any more feature work on the v2 releases, so a minor will not be needed anyways.