orbitjs / orbit

Composable data framework for ambitious web applications.
https://orbitjs.com
MIT License
2.33k stars 134 forks source link

filter seeing inconsistent argument format #874

Closed ef4 closed 3 years ago

ef4 commented 3 years ago

I have a filter method on a RequestStrategy. Sometimes the first argument it receives is formatted like:

{
  "operations": {
    "op": "addRecord",
    "record": {
      "id": "c5439563-2d48-46bf-8bbf-5e39616e96b6",
      "type": "event",
      "attributes": {...}
    }
  },
  "id": "b583d854-907a-4aaa-833f-f21fcbf13853"
}

And sometimes it's formatted like:

{
  "operations": [
    {
      "op": "addRecord",
      "record": {
        "id": "c6665021-9f81-491f-8768-9f5aa3694d57",
        "type": "user",
        "attributes": {...}
      }
    }
  ],
  "options": {},
  "id": "90e8bab4-16b3-49b6-8c41-68a4e9042a66"
}

Notice that in the first one operations is a POJO and in the second it's an array.

Is it intentional that these differ? If it's a bug, I can make a reproduction.

dgeb commented 3 years ago

Ah, sorry for the confusion and breaking change. This is intentional and is explained in the WIP v0.17 docs here. There are subtle breaking changes in the Query and Transform interfaces to bring them into alignment and allow for clearer and more consistent return values (singular vs. arrayed).

In your filter I'd recommend interacting with the transform's operations as an array using this utility:

import { toArray } from '@orbit/utils';

const operations = toArray(transform.operations);

The same approach is recommended for evaluating query.expressions.

ef4 commented 3 years ago

Thanks, the singular vs plural makes sense.

I still have a related ambiguity on query.expressions that is not just singular vs plural. I'm trying to filter queries by type, and I had to check both type and record.type:

return toArray(query.expressions).every(
  (expression) =>
    expression.type !== 'patient' && expression.record?.type !== 'patient'
);
dgeb commented 3 years ago

A record-specific query expression could implement any of these interfaces.

To be fully inclusive of all possibilities you should be aware that expression.records could contain an array of identities. But this check would only be necessary if your application finds records in this way (i.e. q.findRecords([...identities])).

If your purpose is to block certain queries from being forwarded to a source and inspection of the query expressions feels too granular, you might consider using an option that tags your queries in a particular way. For example:

// at the call site
await source.query((q) => q.findRecords('patient'), { remote: 'patientServer' });

// in your strategy
filter(query) {
  return query?.options?.remote === 'patientServer';
}
dgeb commented 3 years ago

Closing, but please comment if anything is unresolved.