Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: storage capacitor #3000

Closed
wants to merge 1 commit into from
Closed

feat: storage capacitor #3000

wants to merge 1 commit into from

Conversation

productdevbook
Copy link

https://capacitorjs.com/

There are errors in my tests, I did not know how to solve it. Can you help with that ?

@kitten
Copy link
Member

kitten commented Mar 5, 2023

Yes, I can, however, please don't use the repository or PRs to test your own packages in the future. This has basically nothing to do with urql except that it happens to implement a type that we accept, basically 😅
While I'm happy to help out with unrelated questions in discussions, this isn't something that's suitable for an issue or PRs. I get that it's convenient, but that's like opening a PR that you know will never be merged as is 😅


The error is very simple. In your mocks you're tryin to do:

      vi.spyOn(Preferences, 'get').mockResolvedValueOnce(
        JSON.stringify({
          [dayStamp]: { foo: 'bar', hello: 'world' },
          [dayStamp - 1]: { foo: 'bar', hello: 'world' },
          [dayStamp - 2]: { foo: 'bar', hello: 'world' },
        })
      );

However, Preferences.get doesn't return a string but a GetResult.
I can see that that's defined as { value: string | null }, so that's potentially a returned structure wrapping around JSON that you may have forgotten? https://unpkg.com/browse/@capacitor/preferences@4.0.2/dist/esm/definitions.d.ts

@kitten kitten closed this Mar 5, 2023
@productdevbook
Copy link
Author

productdevbook commented Mar 5, 2023

This is not my own package, I wrote you a package ?

image

@kitten
Copy link
Member

kitten commented Mar 5, 2023

Well, it is your package, in that you wrote a package that implements our Storage API 😅 You're free to do so and publish it of course, but submitting a PR (or a full package) to our repo means that you're asking us to maintain it.

That's why we have an RFC process for this: https://github.com/urql-graphql/urql/blob/main/CONTRIBUTING.md#how-do-i-propose-changes
Admittedly, I'll update it to make this more clear, but its intention is that if you're aiming to add a new feature, while we may choose to skip the RFC process, there's no guarantee that a change will be accepted.

That's why our PR template points out:

However, if you're starting to work on a large
change, it's best to make sure to open an issue first.

Hence the reason I was a little confused, since you were just asking for help with types, and your PR also doesn't follow our convention. So I was assuming, you were just asking for help.

However, if you're asking for this package to be merged into the repo, maintained by us, and published under this package name, I have to politely declien for now.

Of course it'd be great to have more storage adapters available for Graphcache. However, we can't maintain a larger number of them, and can't guarantee them all to continue working and accept issues for them, especially if we had no hand in writing them or understanding why they're important.
It's basically a maintenance burden that we try to avoid, because often the authors of these larger contributions (like yourself) are the better people to maintain these packages ✌️

@productdevbook
Copy link
Author

productdevbook commented Mar 5, 2023

Hmm, now I get it. Actually I'm using it in capacitor right now and it works great. I thought it was easy for people to npm download used.

Iam not good on the testing vitest side. Key and value had to be entered there. That's why I asked for help. Otherwise the main code is working 99% of the time.

@productdevbook
Copy link
Author

https://github.com/productdevbookcom

We publish it on our side then, can you help us here on the vitest side ?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants