cph-cachet / carp.core-kotlin

Infrastructure-agnostic framework for distributed data collection.
https://carp.cachet.dk/core/
MIT License
20 stars 2 forks source link

Old way of importing Json #196

Closed lorantgulyas closed 3 years ago

lorantgulyas commented 3 years ago

In the Typescript declarations files the current way of importing the kotlinx.serialisation.json.Json is:

declare module 'carp.core-kotlin-carp.common'
{
    import { Long } from 'kotlin'
    import { kotlinx } from 'kotlinx-serialization-kotlinx-serialization-core-jsLegacy'
    import Json = kotlinx.serialization.json.Json
...

However, if I am not mistaken it should be:

declare module 'carp.core-kotlin-carp.common'
{
    import { Long } from 'kotlin'
    import { kotlinx } from 'kotlinx-serialization-kotlinx-serialization-json-jsLegacy';
    import Json = kotlinx.serialization.json.Json
...
Whathecode commented 3 years ago

Which declaration file is this?

From the alpha 22 release notes:

  • JetBrains changed the artifact name in this release, from kotlinx-serialization-core to kotlinx-serialization-json which you need to update in your dependencies.
  • Correspondingly, the kotlinx.serialization TypeScript declarations have been split into two modules: functions to getlist, getset, and getmap serializers remain in core, but to get access to Json you now need to include the new json module. (56eff90)

The get_list_ and get_set_ collection serializer functions have since been renamed to ListSerializer and MapSerializer which need to be used as constructors in alpha 23, but, they remain in kotlinx-serialization-core.

Therefore, you need to import core in addition to json in case you need the collection initializer functions, as demonstrated in the studies unit tests:

import { kotlinx } from 'kotlinx-serialization-kotlinx-serialization-json-jsLegacy'
import Json = kotlinx.serialization.json.Json
import { kotlinx as kotlinxcore } from 'kotlinx-serialization-kotlinx-serialization-core-jsLegacy'
import ListSerializer = kotlinxcore.serialization.builtins.ListSerializer_swdriu$
lorantgulyas commented 3 years ago

As in the example, declare module 'carp.core-kotlin-carp.common' refers to the carp.core-kotlin-carp.common/index.d.ts file.

Whathecode commented 3 years ago

Indeed, seemingly the common, studies, and protocols import the wrong library. Very weird this was not caught in the tests. 🤔

Out of interest, what is the error you were getting? For some reason I have had no reports of this at iMotions.

lorantgulyas commented 3 years ago

It's coming from the compiled carp.core-kotlin-carp.common.js file

Test suite failed to run

    TypeError: Cannot read property 'serialization' of undefined

      10 | import { kotlinx } from 'kotlinx-serialization-kotlinx-serialization-json-jsLegacy';
      11 | import Json = kotlinx.serialization.json.Json;
    > 12 | import { dk as cdk } from 'carp.core-kotlin-carp.common';
         | ^
      13 | import DateTime = cdk.cachet.carp.common.DateTime;
      14 | import UUID = cdk.cachet.carp.common.UUID;
      15 | import EmailAddress = cdk.cachet.carp.common.EmailAddress;

      at node_modules/carp.core-kotlin-carp.common.js:76:88
      at node_modules/carp.core-kotlin-carp.common.js:5:5
      at Object.<anonymous> (node_modules/carp.core-kotlin-carp.common.js:15:2)
      at Object.<anonymous> (src/__tests__/Core.test.ts:12:1)
Whathecode commented 3 years ago

Fixed!

This was not caught by tests because the tests ran using mocha --require ts-node/register which does not seem to care about erroneous imports.

To make sure compilation succeeds now as part of verifying TS declarations, I also run tsc. This correctly caught the error, which I now corrected.