Comments (11)
This is close to what we want (if we expand the default exclusion list):
(defn global-eql-transform
"Returns an EQL transform that removes `(pred k)` keywords from network requests."
[pred]
(fn [ast]
(let [mutation? (symbol? (:dispatch-key ast))
has-children? (seq (:children ast))]
(cond-> (-> ast
(rad-app/elide-ast-nodes pred)
(update :children conj (eql/expr->ast :com.wsscode.pathom.core/errors)))
(and mutation? (not has-children?)) (update :children conj (eql/expr->ast '*))
mutation? (update :children conj (eql/expr->ast :tempids))))))
from fulcro.
Some questions, @awkay, to determine whether adding ::p/errors
anywhere to transactions with mutations is meaningful or not - because it seems that errors of a mutation itself is returned as {<mutation-symbol> {::p/reader-error "some text"}}
.
- Is it so that a "query" and a mutation is never combined in a single transaction / request to pathom, i.e. if the transaction contains a mutation, there are never any non-mutation top-level joins or properties - something like
[:x {:y [:z]} (mutation)]
will never happen and thus adding::p/errors
to the top level is pointless?
(though in RAD there is perhaps<mutation symbol> :com.fulcrologic.rad.pathom/errors
?) - Since mutation errors are returned as reader errors and are returned by default, we do not need to add anything to the transaction to get them in Fulcro
(For mutations, we could perhaps offer a function you can hook into the app's :remote-error?
that would mark mutation reader-errors as errors so that they would appear in the component's ::m/mutation-error
, as produced by the default result action?)
from fulcro.
Honestly I do not remember/know.
In terms of what needs to be in the query: remember that if there is a mutation join that Fulcro uses that for merge as well, so if the key is not in the outgoing query Fulcro itself will filter it.
To be honest I'm torn on how much pathom-specific stuff to include, since that is an external lib that is in the process of changing how it works. I'd probably tend to leave the pathom-specific keys out altogether and document that you might want to customize that part for your app.
from fulcro.
No need for :tempids in mutation queries?!
According to my experiment, we do not need to add :tempids
to mutations:
- If the mutation has no query then Pathom simply returns everything by default, thus including :tempids
- Even If I add a query via m/returning - and the server-seen query becomes
[{(com.example.mutations/create-random-thing {:tmpid #fulcro/tempid["1b59713c-305c-4b8d-802d-7b39b8a4e696"]}) [:fake :com.wsscode.pathom.core/errors]}]
- still tempids are returned to Fulcro
(Pathom 2.3.1, Fulcro 3.5.2)
from fulcro.
Hm. I have seen this mess up in RAD, so I know it doesn't work through the stack. It could be that Fulcro's the one filtering the tempids? Does the remap work in case (2)?
from fulcro.
from fulcro.
Confirmed that in RAD Demo I need to add :tempids to mutation joins otherwise tempids are not returned by Pathom. Need to explore.
from fulcro.
Oh, I see why getting back tempids worked in my first test. I had this in my parser config:
{::p/env {::pc/mutation-join-globals [:tempids], ...}}
@awkay wouldn't adding the Pathom config ☝️ be a simpler solution than modifying the global-eql-transform in Fulcro? It would require more work from the user, namely to copy a "recommended" Pathom setup - but I expect that most users do that anyway? But it seems that neither fulcro-template nor fulcro-rad-demo do this for some reason?
Above you wrote
if there is a mutation join that Fulcro uses that for merge as well, so if the key is not in the outgoing query Fulcro itself will filter it
Does that mean that is :tempids
is not in the outgoing Fulcro query then Fulcro will ignore it, even if it is in the incoming data? But that is not congruent with my experiements so maybe tempids is special, compared to any other query property?
You also wrote
I'd probably tend to leave the pathom-specific keys out altogether and document that you might want to customize that part for your app.
I guess it means you would after all prever not including ::p/errors
in the query, is that correct? If that is the case - what about using a general, Fulcro specific key, such as com.fulcrologic.fulcro.networking.[http-?]remote/errors
, and expecting that the Fulcro server-side glue (or the remote implementation) remaps that to whatever the particular kind of remote uses? So in the case of Pathom, it would be some fulcro-integration-plugin that would move ::p/errors
from the result into this key on the way out?
from fulcro.
Yes, I was not aware of this option in Pathom. It would be better to just recommend that and fix the templates.
In terms of the errors: Fulcro does not require Pathom, and how you deal with errors on Pathom is also pretty configurable. I'm fine with setting up the templates to glue together error handling in some "useful" manner but given that Pathom 3 might even change this key I really don't want to chase Pathom on this in Fulcro's code.
from fulcro.
Then I guess we should close this issue as "won't fix"?
from fulcro.
Yeah, let's open up the templates and fix them, and also document it in the tempids section of the book. Let's just track it with this issue, since all the history is here.
from fulcro.
Related Issues (20)
- eb/error-bondary breaks Fulcro Inspect's element picker
- Unnecessary re-rendering HOT 3
- Denormalize should not "detect loop" for leaf entities HOT 2
- Associate a list of components with a different instance of a state machine each HOT 1
- Mutation params are "closed over" on mutation submission, which causes problem in result actions. HOT 9
- Warnings about missing initial state for anonymous components with a `:componentName` HOT 7
- `comp/get-query` not correct when using raw component and union query HOT 4
- Fix text area and table examples exploding width HOT 2
- Fulcro 3.6 w/React 18
- http error 404 on github-pages HOT 2
- Fulcro 3.6 use of React Context breaks shouldComponentUpdate optimizations HOT 9
- http-remote :status-code is always 500 on error HOT 6
- Potentially add "Accept: application/transit+json" header. HOT 1
- Potentially omitted :force-root in `schedule-render!` HOT 5
- dr/change-route-relative! -- denied route does not call :route-denied component option HOT 1
- Dependency issue with taoensso.encore/dissoc-in HOT 2
- fs/dirty-fields misses recursive field HOT 1
- Link/ident queries don't work with `false` values
- dynamic routing question HOT 1
- HTTP middleware for making load/mutations work easily with JSON http APIs.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from fulcro.