Open klis87 opened 3 years ago
Hi @klis87, i totally understand the motivation here. Questions:
useQuery
syntax would look like
const myQuery = useQuery({ type: someAction.toString() });
?
meta.variables
at point 7? I just š±š±š± discovered the property variables
for useQuery
. Is this related?Answers/opinions:
payload.request
but i like the fact that i know what to expect when i see and handle payload
property. Moving away from a standard results in an increasing learning curve (probably).@iwan-uschka
1) no, just useQuery({ type: someAction })
(instead of useQuery({ type: 'SOME_ACTION' })
), this is already possible in the current version, it is also possible that the next major release will make it as useQuery(someAction, options)
, as type
is necessary always anyway, options are optional though
2) variables
were added recently, see React guide in the docs, variables are useful when autoLoad is true, and your query action accepts some props, then useQuery
will inject them when calling type
callback (to dispatch query action)
3) this is about having better Typescript types and autocomplete in editor for request action, right now we have autocomplete for meta
, which are general options for the core, but request
depends on driver, be it axios config, fetch config, or whatever a driver expects. We could add generic DriverOptions
to createQuery
and its family for example, which could be then set accordingly depending on used driver (like axios config)
Regarding FSA, this is really a tough call, I thought about this because API is growing, lib becomes more and more complex and this could simplify things a lot in many places. Plus, with this library, really you almost don't need to use Redux at all, and my goal is to make it clear that no Redux knowledge is required to use it, but thx to Redux used for the core, apps are easy to debug, we have access to stable ecosystem and we can track any state transition in Redux dev tools for free.
Regarding FSA-only middleware, are you aware of any popular which assumes only FSA action types?
Thanks for your response.
useQuery({ type: someAction })
=> pretty cool
useQuery(someAction)
=> even nicercreateRequest
function to benefit type checking and autocompletion. But i am still not quite sure about whether it interferes with declaring and configuring drivers globally via handleRequests
and createDriver
.Regarding FSA-only middleware, are you aware of any popular which assumes only FSA action types?
Nope :)
handleRequests
and createDriver
I agree with all points, and totally agree that current necessity to provide action type together with action creator (in default setup without help of other action creator libraries) looks very ugly - request creator should gracefully solve that issue.
Just want to say 2 things:
createAction
produces FSA actionvariables
- it's not always possible to know in ahead which variables will be passed into useQuery
or useMutation
hook, so it will be good to have a way to pass variables directly into load
method (that will be true for cases when autoload = false in useQuery
). Example:
const {load} = useQuery({
type: 'IS_EMAIL_REGISTERED',
action: isEmailRegistered,
autoload: false
});
// This is submit handler for form provided by "formik" library: const onFormSubmit = (formValues, formHelpers) => { // Before user registration - check whether user with such email already registered, // and if yes - display proper error below email input: const { data: isEmailAlreadyRegistered } = await load(formValues.email); // <-- HERE IS VARIABLES PASSED if (isEmailAlreadyRegistered) { formHelpers.setFieldError('email', 'Email already registered'); return; }
// ... Email is not registered, so make request here to register user
}
@avasuro thx for the great feedback!
please, don't drop FSA - I know at least one lib
Yeah, also redux-act
and probably redux-actions
also does it. However, wondering whether this is still an issue? If someone uses custom redux actions in FSA style, would it be a problem to support both FSA and not FSA actions?
And, if you see any problems and we would still need to support both, wondering how we should do that. I don't wanna force FSA, but how to make it configurable? createQuery
would need to have an option like useFSA
, but passing it in all places would be terrible. Global config woud be possible, but then it would need to be a global object, which I don't like. It could be supported by:
createQueryFSA
, but I don't really like the longer name...useFSA
oneself, like const createQueryCustom = (type, driverConfig, options) => createQuery(type, driverConfig, options, true)
, but then, one would need to create wrapper for mutations and... incoming subscriptions toosetFsa(true)
, but then the order of imports could wrapper, and with ES6 imports this can lead to unexpected bugsThis is a really tough nut to crack
about variables - it's not always possible to know in ahead
very good example, I assumed that useQuery will rerender on variable change and new memoized load will be used, but indeed, it is not always the case, I will need to make this change!
Adding to this discussion, it is worth mentioning, that even currently all utility actions like resetRequests
, abortRequests
and so on are NOT FSA actions! Just request actions can by of FSA type, because for now there is no official helper to create them. Anyone could create them however one wants, either manually, or by an action creator library like redux-act
. And this was precisely the reason why FSA request actions are supported. But once official helper like createQuery
would be added, then creating request actions will become standardized, hence no need to support them in FSA form anymore. People could still create own Redux actions with any library like toolkit or redux-act, just actions from this library will be not FSA.
To sum up, the only way I could support FSA action is to transform all actions to FSA mode, for example resetRequests
would be:
{ type: 'RESET_REQUESTS', payload: ['QUERY1', 'QUERY2'] }
but really I am not sure we should go FSA or not FSA. However, supporting both would just cause big overhead without too big reasons.
@klis87
I read your previous answers, and there is something to think about, so I will answer later (hope I will not forget about them). But according to this answer:
To sum up, the only way I could support FSA action is to transform all actions to FSA mode, for example
resetRequests
would be...
I think there is no sense to transform internal actions of this library to FSA (or not FSA). When I wrote about that it is worth keeping the support of FSA events, I meant the ability to create actions for requests. Like this library should support both versions of actions:
{type: 'MY_ACTION', request: {url: '/getSomething'}}
and
{type: 'MY_ACTION', payload: { request: {url: '/getSomething'} }}
Because "initial" actions can be created in several ways (using plain object, or using some action creator library, or using some utility function provided by this library) and it is not always easy to produce only non-FSA (or only FSA) actions. While "internal" actions (reset, abort) can be created only via utility functions provided by this library (in theory they also can be created manually, but that is unsupported way, so no need to take it into account, I think).
@avasuro Good points. The question is then, will all people use createQuery, createMutation and createSubscription helpers? In my opinion yes, and in fact this way will be advocated in all examples and documentations. This is the only way I can show nice API in examples, without duplicating action and constant imports in places like useQuery
.
But, if not, if some people preferred not using those helpers, then it would be enough to support fsa request actions in reducers and middleware, helpers themselves wouldn't need to support fsa flag.
There is also one important thing to put in the equasion, I am pretty sure that the next major version will support request actions only created from Redux action creator lib, be it redux-act
or built-in createQuery
. This is because I don't want people to pass both constant and action creator to useQuery
(action
prop will be removed). This even more increases the chance that people will create requests only via built-in helpers. Then, request actions also will become some kind of internal actions. Then, they can be FSA, or not, it doesn't really matter.
Mmm, if I understand right: to simplify library code it will be good to choose some one type of actions (FSA or non-FSA) to use inside this library.
If you force users to use only your helpers (createQuery
etc.) - then as you said it doesn't matter which type of action to use. But if you leave the opportunity for users to choose which way of creating events to choose, then only FSA actions needs to be supported.
Should we force users to use only our helpers to create actions? I don't think that it is good practice (until it is absolutely unavoidable), it's better to leave ability for user to decide how to create actions.
How user can produce actions? I see only 3 possible ways how end user may want to produce actions:
how action can be created | produces FSA or non-FSA actions? |
---|---|
own helpers (e.g. createQuery ) |
can be any |
action creator lib (e.g. redux-act ) |
can be any (in most cases only FSA) |
manually | can be any (the only restriction can be some internal development rules for specific project, which forces to use only FSA actions) |
Pros and cons of choosing FSA/non-FSA
action type to use | cons | pros |
---|---|---|
only non-FSA | - impossible to create such action using most action creator libs (they produce only FSA) - this type may be prohibited for use by internal development rules for specific project |
- ??? |
only FSA | - not backwards compatible with current lib version (at least in all examples in documentation you are using non-FSA actions, so I think most of the users who manually create events will use this type of actions right now) |
- will work in all cases: all action creator libs that produces non-FSA actions can produce FSA actions too, same thing for projects where mostly non-FSA actions used - nothing restrict such users to produce FSA actions for this library |
So, according all above: I think it's better to use FSA actions in this lib, because they will work in all possible use cases. Yes, this approach is not backwards compatible, but because currently library supports both actions types dropping support of any of them is a breaking change, so this con is not so significant from my point of view.
... I am pretty sure that the next major version will support request actions only created from Redux action creator lib, be it
redux-act
or built-increateQuery
. This is because I don't want people to pass both constant and action creator touseQuery
(action
prop will be removed). ...
Fair enough, then you can exclude all things related to "manually created actions" from my previous answer.
@avasuro thx for your analysis, indeed it looks like going FSA all the way could be the best option. Actually we almost have FSA anyway, as we use meta
convention. We could even change request
into payload
without nesting request
in payload
, because probably next version will recognize request actions and queries differently. Probably we will just have a new mandatory meta.requestType: 'QUERY' | 'MUTATION' | 'SUBSCRIPTION'
I know this is quite a big change, but this will be for a reason. Not for all drivers it is possible to deduct whether it is query or mutation. For example, for promise driver it is always necessary to pass meta.asQuery
. This will also simplify many things, for example we could remove isRequestAction
and isRequestActionQuery
from handleRequests
config, as adjusting this won't be necessary anymore - meta.requestType
will tell us all explicitely about the type.
But with createQuery
and family, meta.requestType
will be added just automatically. I think we could also add meta.variables
automatically, then we could use this info for example to make effect takeLeading
instead of takeLatest
for a query by default, because without variables it doesn't really make sense to abort previous pending query and start a new one, it would be a nice optimisation, done automatically for the user. But this requires obligatory usage of createQuery
helpers (in theory one could do this manually, but why?). And frankly, I don't understand why someone would use redux-act etc as createQuery
will do the very same thing, but a little more. But, perhaps FSA should be enforced, as then all will be compatible with createReducer
of many libraries like redux-act
, which require FSA structure.
Also, I have some more plans for action creator helpers - one of my TODO is adding support for offline mutations - they will be queued when offline, and could be auto dispatched when going back online. But callbacks are not serializable, so we cannot just save mutations to local storage for example and recreate them on reload when going online. But I am pretty sure I will be able to mitigate it with some alternatives, thx to nothing else than request creators. This is yet another argument to really enforce usage of this, but in the docs we should of course explain, what it does under the hood, so Redux users will understand what's going on behind the scenes
Right now we create requests like this:
This is problematic due to the following reasons: 1) no autocomplete, unless you use TS and
RequestAction
interface, but with pure JS you won't get autocomplete at all 2) it is just redux action, it requires to have constant and action separately, however often we need both to a function likeuseQuery
,type
is needed for selector and action is needed to be called to automatically fetch 3) issue from point 2 is of course solvable by using action creator library, which adds constant automatically and addstoString
method returning this type, so that action becomes both action itself and source of its type, I mean libraries likeredux-act
,redux-actions
and myredux-smart-actions
4) but 3 is hard to explain to many new Redux developers, and probably just impossible totally for people not knowing Redux, and asking to use constants looks pretty bad, API won't look attractive, and telling people to use action creator library will also look weird, especially the goal is to use this lib even by people not knowing Redux, right now it is already possible with the latest React releaseSo, suggestion? For instance helper like
createQuery
, which could work like that:Then,
fetchBook(1)
will return the same what previous version, butfetchBook.toString() === 'FETCH_BOOK'
, so no constants anymore.What we gain? 5) autocomplete 6) No reason to explain why we need constants for people not knowing Redux 7) We could do nice stuff, like inject
meta.variables
which might be nice source of info, we know also whether request is query or mutation, so we won't need things likemeta.asQuery
anymore 8)createQuery
etc could have some generics, which could be filled with drivers like axios driver, then you could importcreateQuery
from a driver and also have driver config typed and autocompletedThis would be not breakable, current way of writing would also work, but in the next major version: 9) API simplification,
useQuery
won't haveaction
prop anymore, as type will be always action creator, never string 10) I would really remove support for FSA action, nopayload.request
, justrequest
, this really shouldn't matter, and library could be simplifiedBut are there any disadvantages? I cannot see, but maybe I miss some, please write if anything comes to your mind!