deepkit / deepkit-framework

A new full-featured and high-performance TypeScript framework
https://deepkit.io/
MIT License
3.2k stars 123 forks source link

Using an `index.ts` file to export multiple types that would be deserialized causes silent failures #318

Open danfma opened 2 years ago

danfma commented 2 years ago

If you have multiple files like bellow:

data/stat-weight-unit.ts

export type StatWeightUnit = 'lbs' | 'kg';

data/stat-engine-power-unit.ts

export type StatEnginePowerUnit = 'hp';

data/index.ts

export { StatWeightUnit } from './stat-weight-unit';
export { StatEnginePowerUnit } from './stat-engine-power-unit';

test.ts


import { StatEnginePowerUnit } from './data';

console.log(cast('kg'));



The serializer will return `undefined` always, and it will not emit the value, no matter if the serialized value is valid or not.
danfma commented 2 years ago

Detail: it happens only when you have named exports like that. After changing that to export * it started working again.

marcj commented 2 years ago

Can confirm this is a bug. It seems not related to export as the following is wrongly computed.

    enum StatEnginePowerUnit {
        Hp = 'hp',
    }

    enum StatWeightUnit {
        Lbs = 'lbs',
        Kg = 'kg',
    }

    type StatMeasurementUnit = StatEnginePowerUnit | StatWeightUnit;
    const type = typeOf<StatMeasurementUnit>();
marcj commented 2 years ago

oh, wrong issue. This was meant for https://github.com/deepkit/deepkit-framework/issues/317

marcj commented 2 years ago

Tried to replicate it in https://github.com/deepkit/deepkit-framework/commit/8b347fc7e862ce81e5c5be4bbfb7f52fb4075c3a, but wasn't able to do so. Maybe data/stat-weight-unit.ts and the other do not contain the type information. You can try to transpile the files to JS and look inside data/stat-weight-unit.js. If there is no export const __Ω StatEnginePowerUnit: any = [] then something is wrong with the compiler/tsconfig setup

danfma commented 2 years ago

Actually, I don't know if you would be able to fix that... the problem happens when you have an index file that is re-exporting types in a named way.

This will cause the problem:

export { StatWeightUnit } from './stat-weight-unit';
export { StatEnginePowerUnit } from './stat-engine-power-unit';

This will not:

export * from './stat-weight-unit';
export * from './stat-engine-power-unit';

In the first way, the compiler will export only that specific type, but then you have two problems, the type will disappear after the compilation (it's a type definition only), and all those __Ω meta that you define inside that file won't be exported, because they are not listed in the named export.

My guess is that you would be able to fix that by two approaches:

  1. A possible solution would be to force the exportation of the meta when you detect that the user is exporting a named type.
export { StatWeightUnit, __ΩA, __ΩB } from './stat-weight-unit';
export { StatEnginePowerUnit, __ΩC, __ΩD } from './stat-engine-power-unit';
  1. Attach the meta information to the type by using maybe a WeakMap, I don't know... the problem with this approach is that when you have an export of a type definition, like above, you won't really have code generated for it, then there is nothing to attach to.
marcj commented 2 years ago

ah, I see, thanks for the explanation. I think we have to adjust export statements in the compiler to fix that, so that

export { StatWeightUnit } from './stat-weight-unit';
export { StatEnginePowerUnit } from './stat-engine-power-unit';

becomes

export { StatWeightUnit, __ΩStatWeightUnit } from './stat-weight-unit';
export { StatEnginePowerUnit, __ΩStatEnginePowerUnit } from './stat-engine-power-unit';

seems straightforward and easy

danfma commented 2 years ago

By the way, if everything goes well, I will create a type to OpenApi (swagger) mapper using deepkit. Actually, we are still locked to NestJS, but I'm playing with the @deepkit/type module as a first approach. If I'm able to achieve that, then I will publish the library, or maybe, create a fork and send to you, so that you can integrate it with the rest of the library later.

I just need some time to accomplish that.

marcj commented 2 years ago

@danfma ok, cool! You might be interested in https://github.com/hanayashiki/deepkit-openapi

danfma commented 2 years ago

Yay, thank you! Probably, one thing less to do! hahaha :D