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

Possible typing issue w/ Entity Service API & populated relations #19252

Closed
sopwerdna opened this issue Jan 17, 2024 · 21 comments · Fixed by #20370
Closed

Possible typing issue w/ Entity Service API & populated relations #19252

sopwerdna opened this issue Jan 17, 2024 · 21 comments · Fixed by #20370
Assignees
Labels
issue: bug Issue reporting a bug severity: medium If it breaks the basic use of the product but can be worked around source: core:strapi Source is core/strapi package version: 4 Issue related to v4

Comments

@sopwerdna
Copy link

Bug report

Required System information

  • Node.js version: 20.8.0
  • NPM version: 10.2.0
  • Strapi version: 4.17.0
  • Database: Postgresql
  • Operating system: macOS, linux
  • Is your project Javascript or Typescript: Typescript

Describe the bug

We've run into a typescript error upon setting up the project with new versions after the 4.14.x typescript migration changes. In short, I believe this is a result of incorrect or conflicting types coming from calls to the Entity Service API, or objects returned from those calls. Unsure if this is a regression or simply an existing issue that was uncovered by the changes in 4.14.x.

Steps to reproduce the behavior

  1. Add a content type (posts for example, api::post.post)
  2. Add a oneToMany relation from the built in Users table to that content type
  3. In a custom controller, call the Strapi Entity Service update() which states that updating relations follows the same syntax as the REST API and includes an example of the syntax.
  4. Using that syntax gives a type error when using typescript: Type '{ connect: ID[]; }' is not assignable to type 'XOneInput'. Object literal may only specify known properties, and 'connect' does not exist in type 'LongHand'.

Expected behavior

In previous versions, our input to the entity service update(...., data: {...}...) function was typed as XManyInput which allows partial updates and that connect: syntax. Now it's expecting XOneInput even though that relation is definitely a one to many relation and likely should be treated as an array. We're also seeing some other related issues (similar to #18784) which may be a result of the same type conflicts.

Looking closer at the types, I believe this is either a problem in the typescript types for Strapi's entity service API types (in the @strapi/types package), or possibly in the documentation (if there's really a different syntax now). It looks to me like the type on that particular one to many relation should be XManyInput which allows partial updates and that connect: syntax.

Additional context

For further context, this seems to be the file in the strapi types package with the problematic types

type Connect = { connect: ShortHand[] | WithPositionArguments<LongHand>[] };
type Disconnect = { disconnect: ShortHand[] | LongHand[] };
type FullUpdate = Set;
type PartialUpdate = Partial<Connect & Disconnect>;
type XOneInput = ShortHand | LongHand | null;
type XManyInput = ShortHand[] | LongHand[] | null | PartialUpdate | FullUpdate;
type RelationInputValue<TRelationKind extends Attribute.RelationKind.Any> = Utils.Expression.If<
  Attribute.IsManyRelation<TRelationKind>,
  XManyInput,
  XOneInput
>;

Thanks in advance for your help in tracking down the problem here, and fixing the issue if it does turn out to be a problem with Strapi's types. As always, let me know if I can provide any further information or details on the problem here.

@hussain-mustafa990
Copy link

I have also experience this issue on version 4.17.1

@sopwerdna
Copy link
Author

Note - I see that Hussain closed his issue above as it appears it was somewhat different - however the original issue here remains (as in #18784) with content types not working correctly at all when using the entity service API.

@Eventyret Eventyret added issue: bug Issue reporting a bug severity: medium If it breaks the basic use of the product but can be worked around source: core:strapi Source is core/strapi package version: 4 Issue related to v4 labels Mar 14, 2024
@williamatpaper
Copy link

williamatpaper commented Mar 21, 2024

Experiencing this issue on 4.20.4

Trying to use:

const data = {
 name: name,
 relation: {
  set: relations.map((relation) => relation.ID),
 },
};
const record = await strapi.entityService?.create('api::record.record', { data });

works fine, however because the update function has data:partial, I get the error listed when using the update function:

record = await strapi.entityService?.update('api::record.record', 1, { data } );

throws:

Type '{ name: string; relation: { set: any; }; }' has no properties in common with type 'Partial<Input<"api::record.record">>'.ts(2559)

@MarijnFK
Copy link

MarijnFK commented Mar 28, 2024

Same / related issue:

Object literal may only specify known properties, and 'visible' does not exist in type 'Partial<Input<"api::task-status.task-status">>'.

image

Workaround is to use the query engine, but that does not work when creating / adding components in a repeatable component field

@KuSh
Copy link

KuSh commented May 21, 2024

Work around with the Entity Service API : type cast with any or unknown

@Convly
Copy link
Member

Convly commented May 22, 2024

@MarijnFK would you mind sharing the definition for the task-status content-type? I would like to try and reproduce the issue.

@MarijnFK
Copy link

MarijnFK commented May 22, 2024

@MarijnFK would you mind sharing the definition for the task-status content-type? I would like to try and reproduce the issue.

sure, but it's quite big / has alot of relations.
The problem also exists when just creating a content-type with just one field (i.e. 'visible')

{
  "kind": "collectionType",
  "collectionName": "task_statuses",
  "info": {
    "singularName": "task-status",
    "pluralName": "task-statuses",
    "displayName": "TaskStatus",
    "description": ""
  },
  "options": {
    "draftAndPublish": false
  },
  "pluginOptions": {},
  "attributes": {
    "task": {
      "type": "relation",
      "relation": "oneToOne",
      "target": "api::event-task.event-task"
    },
    "event": {
      "type": "relation",
      "relation": "oneToOne",
      "target": "api::event.event"
    },
    "team": {
      "type": "relation",
      "relation": "oneToOne",
      "target": "api::team.team"
    },
    "visible": {
      "type": "boolean",
      "default": true
    },
    "map_visible": {
      "type": "boolean",
      "default": true
    },
    "available": {
      "type": "boolean",
      "default": true
    },
    "unlocked": {
      "type": "boolean",
      "default": true
    },
    "status": {
      "type": "enumeration",
      "enum": [
        "none",
        "pending",
        "warning",
        "correct",
        "failed"
      ],
      "default": "none"
    },
    "answers": {
      "type": "relation",
      "relation": "oneToMany",
      "target": "api::answer.answer",
      "mappedBy": "taskStatus"
    },
    "hints": {
      "type": "component",
      "repeatable": true,
      "component": "games.consumed-hint"
    }
  }
}

@Convly
Copy link
Member

Convly commented May 23, 2024

I failed to reproduce the issue using the latest version (4.24.3).
image
Do you have any additional custom config that could impact this?

@derrickmehaffy derrickmehaffy added the status: can not reproduce Not enough information to reproduce label May 23, 2024
Copy link
Contributor

This is a templated message

Hello @sopwerdna,

Thank you for reporting this bug, however we are unable to reproduce the issue you described given the information we have on hand. Can you please create a fresh project that you are able to reproduce the issue in, provide clear steps to reproduce this issue, and either upload this fresh project to a new GitHub repo or compress it into a .zip and upload it on this issue?

We would greatly appreciate your assistance with this, by working in a fresh project it will cut out any possible variables that might be unrelated.
Please note that issues labeled with status: can not reproduce will be closed in 14 days if there is no activity.

Thank you!

@sopwerdna
Copy link
Author

@Convly @derrickmehaffy It does not appear you followed the reproduction steps in the original bug report. I still encounter the same problem with connect: syntax as reported in the original bug report on version 4.24.3. The reproduction attempt in your picture does not seem to include a relation in the data model, which was the trigger for the original problem myself and @williamatpaper encountered.

@derrickmehaffy derrickmehaffy added status: pending reproduction Waiting for free time to reproduce the issue, or more information and removed status: can not reproduce Not enough information to reproduce labels May 23, 2024
@MarijnFK
Copy link

MarijnFK commented May 24, 2024

i'm trying to reproduce the error in a clean install

i got as far that the plugin "strapi-plugin-navigation": "2.4.0", seems to break it. When i remove it and clean node_modules, dist and .strapi, then regenerate types it works. Install it again and gerenerate types breaks it again.

@cyp3rius
Copy link

It's a dependencies conflict, please use 2.5.0 which is aligned to one of the recent versions of Strapi.

VirtusLab-Open-Source/strapi-plugin-navigation#431 (comment)

@Convly
Copy link
Member

Convly commented May 24, 2024

@Convly @derrickmehaffy It does not appear you followed the reproduction steps in the original bug report. I still encounter the same problem with connect: syntax as reported in the original bug report on version 4.24.3. The reproduction attempt in your picture does not seem to include a relation in the data model, which was the trigger for the original problem myself and @williamatpaper encountered.

I've now tried adding oneToOne, oneToMany, manyToOne, and manyToMany relations and haven't hit the issue.

However, after playing with removing node_modules, types, then re-installing/generating everything, I ended up with the "Object literal may only specify known properties..." error (even without the mentioned plugin installed)

I've reloaded my vs-code and the error went away, and I now have access to everything again. Could it be that the error comes from an invalid vscode state?

Do you have compilation errors when building/starting your application? Have you tried restarting/reloading your vscode?

@sopwerdna
Copy link
Author

This occurs regardless of IDE in a fresh clone of a strapi typescript project that has a single content type with a relation to "plugin::users-permissions.user" and attempts to fetch, populate that relation, then update it. I have confirmed it works fine in javascript. See the following repo with examples of both

https://github.com/sopwerdna/strapi-19252-demo

Clone the repo, go into the javascript project subfolder, and run npm i then npm run build and you will see it works fine. Go into the typescript folder, run npm i, then npm run build and you will see the following errors:

src/index.ts:38:37 - error TS2322: Type '{ id: number; }' is not assignable to type 'ID | WithPositionArguments<LongHand>'.
  Type '{ id: number; }' is not assignable to type 'WithPositionArguments<LongHand>'.
    Property 'position' is missing in type '{ id: number; }' but required in type '{ position: PositionalArguments; }'.

and additionally, trying to access a relation as an array

src/index.ts:43:58 - error TS2339: Property 'users' does not exist on type 'GetValues<"api::task.task", GetNonPopulatableKeys<"api::task.task">>'.

@Convly
Copy link
Member

Convly commented May 24, 2024

Thanks for the reproduction repo, it helps a lot!

I can indeed see the issue (although I don't think #19252 (comment) is related)

image

The only problem I can see here is the longhand notation with positional arguments that needs to have position as optional instead of required.

Once the position property is added (or removed from the interface), it correctly infers the populated fields for the query and everything seems to work well.

image

@MarijnFK
Copy link

@Convly it might not be related indeed. Sofar i have tested several repo's with the navigation-plugin and removed it (and clean cache, re-gen types), then it works again. Re-installing the plugin (again, clear cache and re-gen types) problem re-occurs.

@cyp3rius can you verify if the problem could be related to the user-permissions plugin?

@cyp3rius
Copy link

cyp3rius commented May 24, 2024

That's the native plugin used in navigation plugin (checked at init if you got it enabled).

You're the first person reporting that kind of issue and connecting it with plugin which does not interact at all. Have you checked clean repo with a plugin? My guess it's a "size" of collections you already got in your project and navigation plugin in it complexity doesn't help at all and connection implicates the issue.

@sopwerdna
Copy link
Author

The only problem I can see here is the longhand notation with positional arguments that needs to have position as optional instead of required.

Once the position property is added (or removed from the interface), it correctly infers the populated fields for the query and everything seems to work well.

Thanks @Convly this definitely seems to be the solution. Last time I tried updating my actual project to the latest release, I ran into a few other related TS errors around handling populated relations from the entity service as arrays (off the top of my head, map() and filter() among others). The fact that .length works with this positional argument fix suggests it may resolve the others, but I'm away from my work laptop this long weekend & can't confirm. I'll let you know once I am able to check.

I am curious, is this a documentation issue (i.e. the position argument is meant to be required in the long hand version) or is the fix to update the types to reflect the documentation? Either way I think this resolves my immediate problem. Thank you!

@Convly
Copy link
Member

Convly commented May 27, 2024

I am curious, is this a documentation issue (i.e. the position argument is meant to be required in the long hand version) or is the fix to update the types to reflect the documentation? Either way I think this resolves my immediate problem. Thank you!

This is a bug, not a documentation issue, it's been fixed in #20370 and will be shipped Wednesday in v4.24.4

Sorry for the time it took to track down the issue, and thanks for the reproduction repo 🙂

@Convly
Copy link
Member

Convly commented May 27, 2024

Convly it might not be related indeed. Sofar i have tested several repo's with the navigation-plugin and removed it (and clean cache, re-gen types), then it works again. Re-installing the plugin (again, clear cache and re-gen types) problem re-occurs.

cyp3rius can you verify if the problem could be related to the user-permissions plugin?

Hey, since the issue seems to be unrelated to the original one, could you open a new one if you wish to continue the discussion on your problem? Thanks!

(in your new issue it would be awesome if you could include a reproduction repo or example)

@Convly Convly removed the status: pending reproduction Waiting for free time to reproduce the issue, or more information label May 27, 2024
@Convly Convly self-assigned this May 27, 2024
@sopwerdna
Copy link
Author

sopwerdna commented May 29, 2024

Just as an update @Convly - I attempted to update our project to v4.24.4 and this problem is solved, however I encountered an additional closely related problem. I attempt to use either shorthand or longhand syntax in a call to entity service update(), connecting an additional entry in a oneToMany relation, however I get the TS error:

error TS2322: Type '{ connect: ID[]; }' is not assignable to type 'XOneInput'.
  Object literal may only specify known properties, and 'connect' does not exist in type 'LongHand'.

I can't immediately see why this is the case looking through the @strapi/types package, but it seems to be expecting XOneInput when it should be XManyInput: https://github.com/strapi/strapi/blob/develop/packages/core/types/src/modules/entity-service/params/attributes/relation.ts#L26. I will open a fresh issue for this but I wanted to mention it here since it appears to be closely related. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug severity: medium If it breaks the basic use of the product but can be worked around source: core:strapi Source is core/strapi package version: 4 Issue related to v4
Projects
Status: Fixed/Shipped
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants