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

Why does it reset to the first handler when the array is empty? #2410

Closed
1 task
nayounsang opened this issue Jan 11, 2025 · 4 comments
Closed
1 task

Why does it reset to the first handler when the array is empty? #2410

nayounsang opened this issue Jan 11, 2025 · 4 comments
Labels
feature scope:browser Related to MSW running in a browser

Comments

@nayounsang
Copy link

nayounsang commented Jan 11, 2025

Scope

Adds a new behavior

Compatibility

  • This is a breaking change

Feature description

I understand initializing when the argument is undefined, but I think initializing it even when it is an empty array is excessive.
It seems troublesome for developers to classify each case.
If the argument is dynamic, it may lead to unexpected behavior.
(However, it seems difficult to distinguish this technically. Is there a good way?)

const enableMyHandlers = (handlers) => {
  // hmm... handlers.length could be 0. In this case, I don't think there should be any handlers active.
  worker.resetHandlers(...handlers) 
}

const initHandlers = () => {
  // Unlike the above function, this is to obtain the first handler.
  worker.resetHandlers()
}
  public reset(nextHandlers: Array<RequestHandler | WebSocketHandler>): void {
    this.handlers =
      nextHandlers.length > 0 ? [...nextHandlers] : [...this.initialHandlers]
  }

// ...

  public resetHandlers(
    ...nextHandlers: Array<RequestHandler | WebSocketHandler>
  ): void {
    this.handlersController.reset(nextHandlers)
  }

If I can clear the handler, it may be a bit inconvenient, but I think it can be done anyway.
Well, first, let’s create a PR this way. If it's strange, let's discuss another method!

if (someCondition) {
  worker.clearHandlers();
}

// HandlerController
// ...
  public clearHandlers(): void {
    this.handlers = []
  }
}
@kettanaito
Copy link
Member

kettanaito commented Jan 21, 2025

Hi, @nayounsang. Can you please explain what you are trying to achieve here?

Do I understand it right that since nextHandlers.length will be 0 in the case of .resetHandlers() call, the code will put [...this.initialHandlers] back as the active handlers while you want zero handlers to be active?

One way you can get this behavior even now is relying on .use() instead of the base handlers:

-const worker = setupWorker(...handlers)'
+const worker = setupWorker()
+worker.use(...handlers)

This won't treat handlers as initial handlers, so calling worker.resetHandlers() will remove them entirely or replace them with a list of newly provided handlers.

I may even argue that this usage is more correct. Initial, or happy path handlers, aren't meant to be removed. They can be replaced as if to say "now I have a different set of happy path handlers", but removing them entirely may lead to unexpected behaviors if you call .resetHandlers() accidentally in a test.

@kettanaito kettanaito added the scope:browser Related to MSW running in a browser label Jan 21, 2025
@kettanaito
Copy link
Member

We can treat it as a documentation issue though. I don't mind adding this explanation and a usage example to https://mswjs.io/docs/api/setup-worker/reset-handlers.

@nayounsang
Copy link
Author

Hi. Thx for comment @kettanaito.
What i want to achieve
I'm trying to create an msw dev tool.
While creating a feature to enable/disable only the handlers the user wants in real time and selectively, I encountered this issue in the case of disabling all handlers.
I tried worker.stop() or start(), but there is no way to know whether the worker is currently active or not, and looking at the internal code, I thought it was difficult to figure out. (This is just my opinion as I am new to msw!)
Rather than changing the behavior of resetHandlers() to temporarily disable all handlers, I felt it would be simpler to add a new method.
But, I'm going to try this code to solve this problem.

+const worker = setupWorker()
+worker.use(...handlers)

So, I'm thinking of closing the existing (temporary) my PR: #2411 .

@nayounsang
Copy link
Author

I'll try to document it based on the contents of this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature scope:browser Related to MSW running in a browser
Projects
None yet
Development

No branches or pull requests

2 participants