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

Persisting data across multiple requests & service worker updates #254

Closed
parkjoon opened this issue Jun 30, 2020 · 16 comments
Closed

Persisting data across multiple requests & service worker updates #254

parkjoon opened this issue Jun 30, 2020 · 16 comments

Comments

@parkjoon
Copy link

Is your feature request related to a problem? Please describe.
Suppose I create an account (via the mocks) and want to log out and log back in as the new user.
Is there an elegant way to persist that newly created data using MSW?

Describe the solution you'd like
I think whatever solution should be agnostic to the storage method used, whether it's a plain variable, an in-memory DB, or something else. I was thinking the most agnostic and simplest way to go about doing this is just to access whatever storage you may have when you execute your logic in the MSW routes (ex. inside rest.get handler).
However, with the above approach, when the MSW worker is updated (ex. via Webpack hot reload), that state would be wiped in some cases.
Another approach to avoid the data reset would be to leverage the browser cache (via service worker, such as IndexedDB), but that wouldn't be available on server side, which would be another problem.

Was hoping to get some thoughts on how to solve the issue of a persistent data store while using MSW mocks.

@kettanaito
Copy link
Member

kettanaito commented Jun 30, 2020

Hey, @parkjoon! That's a great topic.

As a part of work-in-progress REST API Auth tutorial, I'm using sessionStorage as a persistency layer to store my authentication. As you've rightfully noticed, while that'd work in a browser, it's unlikely to function in Node.

I'm looking for a suitable API (if any) to handle persistency on the library's side. Most likely it would reside in its own package, as it may utilize such concepts as data modeling and relationship mapping as a part of the persistency layer. Please, if you have an idea of how such an API may work with MSW, share it with me, I'd love to hear it.

Since request handlers don't get re-evaluated on hot updates, you should be safe in introducing your own persistence layer that would work for both client and server.

@Aprillion
Copy link
Contributor

Aprillion commented Dec 4, 2020

Here are my thoughts if it helps:

While sessionStorage and localStorage are not available in Node's global, libraries like jsdom already provide these window objects, so that should be covered for persistence done by client code in the tests.

Persistence between Node sessions should be out of scope - we need reproducible deterministic tests, not sharing anything between 2 test runs (and we can use Express if we need a Node server for anything else than tests) => some local variable should be enough in Node.

If possible, the same API as localStorage would be nice => mswStorage.set('a', JSON.stringify({gg: 123}))

Something like following (though I have no idea if MSW can actually use localStorage or only CacheStorage available in the service worker):

function mswStorage() {
  let nodeStorage = {}
  return {
    get: (key) => isNode ? nodeStorage[key] : localStorage.get(`msw-${key}`),
    set: (key, value) => isNode ? nodeStorage[key] = value : localStorage.set(`msw-${key}`, value),
  }
}

Not sure about cookies - if MSW handler sets a cookie, e.g. ctx.set('Cookie', ...), the browser would include the cookie in subsequent requests... But what happens with fetch libraries for Node, do they remember cookies? If not, should it be up to MSW to remember the cookie and pretend that the client included it in subsequent requests? Maybe a dedicated method like ctx.setCookieToIncludeInFutureRequests()?

@kettanaito
Copy link
Member

libraries like jsdom already provide these window objects

While they may polyfill such objects, they may omit their functionality. If localStorage.set is available, doesn't mean that it actually saves the data somewhere in JSDOM (I may be wrong about this).

We've started a concept of a persistency layer that would work in both browser and NodeJS, but it needs some time to evolve and land as a separate package.

Not sure about cookies

Cookies persistency and assignment based on the requested domain is currently in progress in #435. There should be no breaking changes to the API once that support lands.

@kettanaito
Copy link
Member

kettanaito commented Dec 5, 2020

@Aprillion here's the Live Storage repository that we are planning to suggest as a standalone persistency layer to MSW users. It uses sessionStorage to persist the data client-side, and a singleton class to do that in NodeJS. It also leverages BroadcastChannel, so that data updates are synced between all active clients (something alike websocket via browser API).

import { LiveStorage } from '@msw/live-storage'
import { setupWorker, rest } from 'msw'

const posts = new LiveStorage<Post[]>([])

rest.post('/posts', (req, res, ctx) => {
  // Updating the storage syncs the data for the current client
  // and all other active clients, so subsequent calls to POST /posts
  // will operate on the latest populated storage. 
  posts.update((prevPosts) => prevPosts.concat(req.body))

  // Respond with the end value of the entire storage.
  return res(ctx.json(posts.getValue()))
})

I'd be thankful if you gave your opinion on that repository.

@Aprillion
Copy link
Contributor

Aprillion commented Dec 6, 2020

Library looks good generally, though in the example above posts object vs post method are a bit confusing, what about "articles" object? ;)

Also, 1 point about adding listeners in a constructor - if you care about Concurrent Mode safety in React - I understand we would not expect people to create multiple instances, but if it gets popular, someone will useState(() => new LiveStorage([])) sooner or later:

  • Following line in the constructor is not safe for Concurrent mode (even in Strict mode, the constructor would be called twice):
this.channel.addEventListener(

See facebook/react#20090 for a very long discussion on the topic.

A quick fix would be to provide a desctructor method that would this.channel.removeEventListener(...) - which would be usable for anyone who needs multiple instances for whatever reason.

A proper fix for the React use case (but more complicated API when we only need a single global instance in tests) would be to provide a connect/disconnect pair of methods that would addEventListener and removeEventListener, as suggested in mrdoob/three.js#20575 for another library.

@Aprillion
Copy link
Contributor

ah, the constructor takes 2 arguments, so the examples in README and above need to be updated before publishing => new LiveStorage('posts', [])

@kettanaito
Copy link
Member

@Aprillion, a good point. Would you mind issuing a pull request to fix that in the README?

@Aprillion
Copy link
Contributor

@Aprillion, a good point. Would you mind issuing a pull request to fix that in the README?

Will do later ~today..

@kettanaito
Copy link
Member

if you care about Concurrent Mode safety in React

I'd discourage live storage usage in UI in general. The package is designed to store data in a data layer—something you'd declare once and access only through HTTP requests: HTTP request -> MSW -> LiveStorage -> data -> response. Not sure if the Concurrent mode in React is something to be worried about.

@Aprillion
Copy link
Contributor

Aprillion commented Dec 6, 2020

PR for readme created (and looks linked correctly above...)

I also tried to add a test case for the readme example - but the test fails, because BroadcastChannel is not available in Node nor jsdom => see https://github.com/mswjs/live-storage/compare/master...Aprillion:test?expand=1

@kettanaito
Copy link
Member

Thank you for the pull request. Yes, the library currently doesn't support NodeJS, and that's something we could add. In NodeJS the LiveStorage class can be a singleton, storing all the data. That should suffice for persistence.

@Aprillion
Copy link
Contributor

Thank you for the pull request. Yes, the library currently doesn't support NodeJS, and that's something we could add. In NodeJS the LiveStorage class can be a singleton, storing all the data. That should suffice for persistence.

Should I make a PR from my test branch (with a failing test as a kind of TDD), or would you rather wait until someone (maybe me, but not today) updates the class to work in Node before making any tests?

@kettanaito
Copy link
Member

A failing test should come first. I’d be thankful for a draft PR with it.

@kettanaito
Copy link
Member

This is going to be addressed in a separate library. Stay tuned for the upcoming announcements.

@timsofteng
Copy link

@kettanaito Has persistent been implemented?

@kettanaito
Copy link
Member

@timsofteng, no, I never got time for it. If you have some, feel free to create a persistence library for MSW!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants