xapijs / xapi

Communicate over xAPI using JavaScript.
https://www.xapijs.dev/xapi-wrapper-library
MIT License
43 stars 21 forks source link

[Types] StatementObject should not use type Statement but new type SubStatement #322

Closed FunkMonkey closed 1 year ago

FunkMonkey commented 1 year ago

Currently StatementObject is a union that includes Statement, which does not provide objectType. This leads to TS not knowing statement.object.objectType, which is necessary for properly determining the correct object type (even if it is just an optional property as in Activity).

As per xAPI specification a SubStatement should be used though:

A SubStatement MUST specify an "objectType" property with the value SubStatement.

A new interface SubStatement should be added and used for StatementObject, e.g.

export interface SubStatement extends Statement {
  objectType: "SubStatement"
}
export type StatementObject =
  | Activity
  | InteractionActivity
  | ObjectiveActivity
  | Actor
  | StatementRef
  | SubStatement;

Thanks!

CookieCookson commented 1 year ago

@FunkMonkey thank you for using xAPI.js and for raising this issue!

You're right, the usage of a Statement within a Statement object should be a SubStatement and not just a standard Statement, with the inclusion of the objectType property. The spec also identifies that it should not contain the id, stored, version or authority properties or a nested SubStatement. I will see what I can do to best represent this in the Interfaces.

CookieCookson commented 1 year ago

@FunkMonkey This should now be available in 2.2.1, please reopen the issue if this doesn't work as expected.