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

Support excluded paths. #20

Open
darkstarx opened this issue Nov 4, 2024 · 3 comments
Open

Support excluded paths. #20

darkstarx opened this issue Nov 4, 2024 · 3 comments

Comments

@darkstarx
Copy link
Contributor

darkstarx commented Nov 4, 2024

Hi! Thanks for the great package.

I've noticed that if watchDependencies is true, there is the path ./ in the watch list. I'd prefer to exclude this path from the watch list but there is no such a way. Using onBeforeReload is not a good solution in this case, since due to debouncing there may be a batch of events, and some of them should cause hot reloading, but if at least one event is discarded by the onBeforeReload, all other events will be discarded:

      var noVeto = true;
      if (_onBeforeReload != null) {
        if (changes?.isEmpty ?? true) {
          noVeto = _onBeforeReload?.call(new BeforeReloadContext(null, isolateRef)) ?? true;
        } else {
          for (final change in changes ?? <WatchEvent>[]) {
            if (!(_onBeforeReload?.call(new BeforeReloadContext(change, isolateRef)) ?? true)) {
              noVeto = false;  // <<< HERE a single event discards all the batch of events.
            }
          }
        }
      }

I would change the logic here so that the onBeforeReload becomes just a filter for events. If no events remain in the batch, we don't reload the code, otherwise we assume some other (not discarded) events cause hot reloading and launch the process.

What do you think?

@sebthom
Copy link
Member

sebthom commented Nov 4, 2024

Sounds good!

@darkstarx
Copy link
Contributor Author

May I make a PR on changing the logic of applying the _onBeforeReload?

@sebthom
Copy link
Member

sebthom commented Nov 4, 2024

Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants