aas-core-works / aas-core3.0-typescript

Manipulate, verify and de/serialize asset administration shells in TypeScript.
Other
1 stars 0 forks source link

Importing types without module name prefix #16

Closed milofranke closed 10 months ago

milofranke commented 10 months ago

To use a type from types.ts, right now you would have to import all types like this: import { types } from '@aas-core-works/aas-core3.0-typescript' and then get the specific type you want to use like this: types.AssetAdministrationShell.

Instead I would like to import specific types directly like this: import { AssetAdministrationShell } from '@aas-core-works/aas-core3.0-typescript' and just use the type without the types. prefix.

For that, the aliases in the index.ts file would have to be removed. Would that be possible or is there any reason not to do that? Thanks in advance!

mristin commented 10 months ago

Hi @milofranke , Thanks for reaching out with the suggestion!

Apart from cosmetics, is there any reason why you would want to have it that way? I followed more or less the design of all the other AAS SDKs.

milofranke commented 10 months ago

Hi @mristin , thanks for the quick response!

Besides the cosmetics, I think it would make using the types (and also functions from other modules) less tedious. Also, we have about 100 usages of the types in our project right now. Because we copied the source code directly into our project, it wasn't an issue, but as we want to switch to the npm package now, we would have to prefix all the types.

So as long as it doesn't cause any conflicts, this change would be highly appreciated!

mristin commented 10 months ago

Hi @milofranke , I do understand your pain point, thanks for clarifying!

Another reason, which only came to my mind later, why there are prefixes are name collisions. For example, if IDTA introduces a new type, we have to make sure that it does not collide with anything in verification, jsonization and other modules. Hence, allowing types to enter the root package space would cause us to revert it on such a likely change in the future. Therefore, I am reluctant to make the change.

I would like to stress it again: I do see your point, and it makes writing client code more tedious.

On a side note, couldn't you simply import the types that you want as:

import {
  AssetAdministrationShell
} from '@aas-core-works/aas-core3.0-typescript/types'

?

milofranke commented 10 months ago

Hi @mristin ,

I understand your point with the name collisions and it's probably a good reason not to do it as I initially proposed. To import the types like you suggested would also be a good solution, but as far as I understand, you would have to explicitely allow to import from the types module in the export section of the package.json.

So it should look something like this:

"exports": {
    "require": "./dist/lib/cjs/index.js",
    "import": "./dist/lib/esm/index.js",
    "./types": "./dist/types/types.d.ts"
 }
mristin commented 10 months ago

@milofranke Would you mind creating a pull request?

@eboxberger could you please also test the snippet on your side?

I'll try to test it today on my side as well.

mristin commented 10 months ago

@milofranke I've just created a pull request. Once we merge it in, please let me fix #15 before I publish the version 1.0.0 (i.e., dispense of release candidates).

treibholzhq commented 9 months ago

A release of this would be really important because the lib is unusable otherwise. image

mristin commented 9 months ago

@treibholzhq we have just released the first stable version, https://github.com/aas-core-works/aas-core3.0-typescript/releases/tag/v1.0.0. It should be available on npm shortly.