chore(TS): fix submodule type import

[jiayihu]

The following code that imports a type from a submodule doesn't work as consumer of fabric.

import { GraphemeBBox } from "fabric/dist/src/shapes/Text/Text";
import type { GraphemeData } from "fabric/dist/src/shapes/Textbox";

(This was written by you @ShaMan123 ). I believed initially this was caused by our module augmentation, because VSCode would always open the augmentation file (with declare module "fabric") along with fabric/dist/index.d.ts. Indeed I said multiple times in other issues that submodule type imports don't work with module augmentation, but it seems I was wrong.

Today I found out that if I remove typesVersions, the above imports work. I think it's because the typesVersions is resolving any (i.e. *) import to dist/index.d.ts:

• fabric -> dist/index.d.ts
• fabric/dist/src/shapes/Text/Text -> dist/index.d.ts

It should probably be *: [dist/*] and "types": "./index.d.ts" as described in the official doc. For now, I've just removed the config since I'd like to understand the initial rational as well.

Ironically, I found out your comment @ShaMan123 in microsoft/TypeScript#8305 (comment) while I was investigating why the submodule type import did not work. I didn't understand however the rational for typesVersions. After I removed, I was still able to compile fabric fine with build:fast.

[ShaMan123]

This is the only way I found to instruct TS of mutliple entry points of the build so a consumer has types for each entry point.
Fabric has a node entry point, removing the node field kills type detection for it.
I decided to keep src/* but I should have removed it.
Same for *, I didn't find a better way. Perhaps we just need to change the order. Have node first and then *.
I don't want to maintain d.ts files if possible (I prefer other solutions), this solution should be tweaked to work unless TS came up with a proper solution by now.

[jiayihu]

Fabric has a node entry point, removing the node field kills type detection for it.

How do I check that the node entry point is correctly typed? Like this?

import { StaticCanvas } from "fabric/node"

new StaticCanvas().getNodeCanvas() // <= should not error?

[ShaMan123]

Yes

[ShaMan123]

A better solution could be dropping the dist folder in favor of node, web folders.
Then TS resolves them automatically using FS and not with this config.
Add to that removing the src folder in the publish action and we cover all bases.
It is also straight forward for a dev to look around.

[asturur]

I m not sure we support those imports

import { GraphemeBBox } from "fabric/dist/src/shapes/Text/Text";
import type { GraphemeData } from "fabric/dist/src/shapes/Textbox";

the correct import should always be:

import { GraphemeBBox } from "fabric";
import type { GraphemeData } from "fabric";

no?

[ShaMan123]

Yes

[jiayihu]

Generally speaking yes, but you would need to export every type from any module. Sometimes, for pretty internal exports, it's common in the TS community to import a submodule, even more for a type. For instance, in React, you often do things like:

import {TextBlock, MediaBlock, TextRow, RectShape, RoundShape} from 'react-placeholder/lib/placeholders'

From https://github.com/buildo/react-placeholder#customization, when you want to create your own placeholder. So it's not the most common use case, more an advanced one. Similar to this case where we want to know some grapheme info from fabric for our own font resizing logic.

Of course, as consumer, you're aware that you're importing a type from an internal module and it might break in future. It's an acceptable compromise, because otherwise as fabric maintainer, you need to export the type AND to maintain it. In this case for instance, you might not want to export GraphemeData because that would make it officially a public type, thus to be maintained.

That's just to say it's not an anti-pattern to import a subtype module, especially to implement some low-level logic. In this specific case, if you prefer to export publicly GraphemeData and GraphemeBBox from fabric, I'm of course fine with it and I'll update the PR. But surely, later in future, there will be the need for other internal types.

[ShaMan123]

I am for exposing as much as possible

[asturur]

i m against exposing submodules because @ShaMan123 has proved himself prone to changing files names and file cases often.
I can barely control an interface i can't control a file tree.

[ShaMan123]

And that is after you tamed me

[ShaMan123]

Moving forward...
I think we should close this and simply export all relevant types