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

Change Tokens and Duplicate OnChange firing in rapid succession #36045

Open
dazinator opened this issue Feb 27, 2020 · 5 comments
Open

Change Tokens and Duplicate OnChange firing in rapid succession #36045

dazinator opened this issue Feb 27, 2020 · 5 comments
Labels
area-Extensions-Primitives documentation Documentation bug or enhancement, does not impact product or test code help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@dazinator
Copy link

I'm opening this issue again, as per guidance from @guardrex

The reasons for me opening this again, are that the issues linked above to track the problem have been closed by engineering, however:

  • The problem has been acknowledged, but still exists unresolved - i.e no framework provided solution or workarounds. This also mean no adequately tested / officially approved workarounds.

  • Developers won't expect an API named OnChange to fire twice in rapid succession when there is a single change to a file. So I think this is still very likely to cause developers issues when using this API.

  • Without any official debouncing methods that are readily adoptable to developers who encounter this, they will have to spend time rolling (and testing) their own solutions. This is a time sink that would be much more efficiently fixed at the source by the framework providing a couple of approved and tested methods for this. The documented workaround provided by @guardrex may help developers to a limited degree, but it is limited because it has no real tests so could exhibit bugs, and sha1 checking may cause other problems later when the application is scaled out or large files are introduced - so could actually lead them down a bad path without these considerations being pointed out. Also the documented sample code is not in a form that is readily adoptable and maintainable in a central location - if for example bugs are discovered, then each developer will most likely have their own copied and pasted version to maintain.

  • The approach in the gist with the configurable delay is probably much more robust than the documented sha1 checking approach, as long as you configure a sensible delay.

  • I don't believe it would take that much effort on the part of engineering to provide some framework approved debouncing implementations to resolve this issue, either including them as optional API's in the same package as ChangeToken itself or an optional support package, and then update the docs to document each of those. The gist I feel is probably the most broadly applicable'/ widely useful solution to this - with barely any dependencies at all, and could probably more easily be included out of the box than something like the sha1 check though imho.

I appreciate you want to get on and deliver other features. Users of the framework are like-minded in that regard.

Describe the bug

The bug is already well described in the issues linked above.

To Reproduce

See linked issues.

Further technical details

See linked issues.

@guardrex
Copy link

Thanks @dazinator and @cocowalla. At the very least, I'll improve the doc coverage:

  • Improve the coverage of this scenario.
  • Mention that the approaches shown ... file hashes (duplicate handling) and an exponential back-off (delay handling) ... won't address every scenario.
  • Mention (possibly cross-link) community approaches and/or engineering issues.

I'll open an issue and react when this is resolved here. 👂

@Pilchie Pilchie transferred this issue from dotnet/aspnetcore Feb 27, 2020
@Pilchie
Copy link
Member

Pilchie commented Feb 27, 2020

Just a note that I transferred this to the extensions repo

@analogrelay analogrelay transferred this issue from dotnet/extensions May 7, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label May 7, 2020
@analogrelay analogrelay added this to the Future milestone May 7, 2020
@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Jun 22, 2020
@maryamariyan maryamariyan added the documentation Documentation bug or enhancement, does not impact product or test code label Sep 30, 2020
@maryamariyan
Copy link
Member

@guardrex I was wondering if there is a PR on dotnet-api-docs that is linked to this issue?

@maryamariyan maryamariyan added the help wanted [up-for-grabs] Good issue for external contributors label Sep 30, 2020
@guardrex
Copy link

Not that I'm aware of. There's only the original docs (ASP.NET Core) issue at ...

dotnet/AspNetCore.Docs#9785

... before it was closed there and opened for engineering (then transferred here).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-Primitives documentation Documentation bug or enhancement, does not impact product or test code help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

6 participants