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

Updated json2typescript to 1.4.1 #160

Merged
merged 14 commits into from
Apr 15, 2020

Conversation

andreas-aeschlimann
Copy link
Contributor

Closes #159

@tobiasschweizer
Copy link
Contributor

@andreas-aeschlimann Is this ready for review?

@tobiasschweizer tobiasschweizer added the bug Something isn't working label Mar 11, 2020
@tobiasschweizer
Copy link
Contributor

With this PR, full class inheritance should be possible also with incompatible type annotations on different subclasses / siblings.

@andreas-aeschlimann
Copy link
Contributor Author

I locally ran the tests and it was fine, I don't know why it failed here.

@tobiasschweizer
Copy link
Contributor

I locally ran the tests and it was fine, I don't know why it failed here.

That was a general issue, should be fixed now.

@tobiasschweizer
Copy link
Contributor

@andreas-aeschlimann I would like to try to rebuild my original class hierarchies on this branch to see if it works now.

@andreas-aeschlimann andreas-aeschlimann changed the title Updated json2typescript to 1.2.5 Updated json2typescript to 1.3.0 Apr 13, 2020
@andreas-aeschlimann
Copy link
Contributor Author

2 tests are failing since the upgrade to json2typescript 1.3.0. I could not figure out yet why this happens @tobiasschweizer

@tobiasschweizer
Copy link
Contributor

ValuesEndpoint
1508
Method createValue
1509
✖ should create an interval value
1510
HeadlessChrome 80.0.3987 (Linux 0.0.0)
1511
Error: Expected $['http://0.0.0.0:3333/ontology/0001/anything/v2#hasInterval'] to have properties
1512
http://api.knora.org/ontology/knora-api/v2#intervalValueHasEnd: Object({ @type: 'http://www.w3.org/2001/XMLSchema#decimal', @value: '3.4' })
1513
http://api.knora.org/ontology/knora-api/v2#intervalValueHasStart: Object({ @type: 'http://www.w3.org/2001/XMLSchema#decimal', @value: '1.2' })
1514
at
1515
at UserContext. (src/api/v2/values/values-endpoint.spec.ts:1536:36 <- src/api/v2/values/values-endpoint.spec.js:1760:48)
1516
at
1517

1518
ResourcesEndpoint
1519
method createResource
1520
✖ should create a resource with values
1521
HeadlessChrome 80.0.3987 (Linux 0.0.0)
1522
Error: Expected $['http://0.0.0.0:3333/ontology/0001/anything/v2#hasInterval'] to have properties
1523
http://api.knora.org/ontology/knora-api/v2#intervalValueHasEnd: Object({ @type: 'http://www.w3.org/2001/XMLSchema#decimal', @value: '3.4' })
1524
http://api.knora.org/ontology/knora-api/v2#intervalValueHasStart: Object({ @type: 'http://www.w3.org/2001/XMLSchema#decimal', @value: '1.2' })
1525
at
1526
at UserContext. (src/api/v2/resource/resources-endpoint.spec.ts:246:36 <- src/api/v2/resource/resources-endpoint.spec.js:292:48)
1527
at

@tobiasschweizer
Copy link
Contributor

@andreas-aeschlimann
Copy link
Contributor Author

I think that's due to a missing decorator in

https://github.com/dasch-swiss/knora-api-js-lib/blob/6520ea63b30ae201996f3e59f6d9d7035858022a/src/models/v2/resources/values/create/create-interval-value.ts#L7

certainly my fault, I will create an issue for it

Interesting, because seconds ago I opened this issue: appvision-gmbh/json2typescript#129

In my older projects, I had errors too and found simply that properties were ignored of child classes when the decorator was missing.

@tobiasschweizer
Copy link
Contributor

I think that's due to a missing decorator in
https://github.com/dasch-swiss/knora-api-js-lib/blob/6520ea63b30ae201996f3e59f6d9d7035858022a/src/models/v2/resources/values/create/create-interval-value.ts#L7

certainly my fault, I will create an issue for it

Interesting, because seconds ago I opened this issue: AppVision-GmbH/json2typescript#129

In my older projects, I had errors too and found simply that properties were ignored of child classes when the decorator was missing.

:-)

please have a look at #173

@tobiasschweizer
Copy link
Contributor

@andreas-aeschlimann Just let me know once there is a version 1.3.1

@tobiasschweizer
Copy link
Contributor

@andreas-aeschlimann Will the issue with incompatible decorators be fixed as of 1.3.0 (1.3.1)?

What is 1.2.5?

@andreas-aeschlimann
Copy link
Contributor Author

I recommend updating to v1.4.1. It finally

  • should be possible to use with TS < 3.8 (Angular < 9) as well
  • forces you to use the decorator with at least 1 parameter

The reason for this sudden version bump:

  • v1.3.0 allowed you to use 0 or 1 parameter in the decorator, it implemented new features but it was incompatible with TS < 3.8 (Angular < 9).
  • Hence I increased to v1.4.0 because it contained a breaking change (1 decorator parameter is enforced)
  • I had to update to another version v1.4.1 because there was still another incompatibility with TS 3.8 (Angular <9).

@andreas-aeschlimann andreas-aeschlimann changed the title Updated json2typescript to 1.3.0 Updated json2typescript to 1.4.1 Apr 14, 2020
@andreas-aeschlimann
Copy link
Contributor Author

Updated to 1.4.1. Tests are passing locally, so this should be good to go.

@tobiasschweizer
Copy link
Contributor

Updated to 1.4.1. Tests are passing locally, so this should be good to go.

Ok, I will try this today. Thanks!

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Apr 15, 2020

@andreas-aeschlimann I could re-establish the class hierarchy that I had to limit because of a bug in a previous version of json2typescript.

Could you confirm that an inheritance like the following is correct:

BaseValue (@type) -> WriteValue -> UpdateValue and CreateValue

BaseValue:

@JsonProperty("@type", String)
    type: string = "";

So all subclasses of BaseValue inherit the decorator for @type.

Also it should now be possible to define a decorator like

WriteValue -> BaseValue

@JsonProperty(Constants.HasPermissions, String, true)
    hasPermissions?: string = undefined;

and

ReadValue -> BaseValue

@JsonProperty(Constants.HasPermissions, String)
    hasPermissions: string = "";

on siblings without conflicts.

@andreas-aeschlimann
Copy link
Contributor Author

@andreas-aeschlimann I could re-establish the class hierarchy that I had to limit because of a bug in a previous version of json2typescript.

Could you confirm that an inheritance like the following is correct:

BaseValue (@type) -> WriteValue -> UpdateValue and CreateValue

BaseValue:

@JsonProperty("@type", String)
    type: string = "";

So all subclasses of BaseValue inherit the decorator for @type.

Also it should now be possible to define a decorator like

WriteValue -> BaseValue

@JsonProperty(Constants.HasPermissions, String, true)
    hasPermissions?: string = undefined;

and

ReadValue -> BaseValue

@JsonProperty(Constants.HasPermissions, String)
    hasPermissions: string = "";

on siblings without conflicts.

Yes, this should work without problems now.

@tobiasschweizer tobiasschweizer merged commit 7019ad2 into master Apr 15, 2020
@tobiasschweizer tobiasschweizer deleted the wip/159-update-json2typescript branch April 15, 2020 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update json2typescript
2 participants