kontent-ai / model-generator-js

MIT License
13 stars 9 forks source link

Update structure of contentTypes to support tree shaking #44

Closed itworksafisher closed 1 year ago

itworksafisher commented 1 year ago

Motivation

Why is this feature required? What problems does it solve?

We use the smart link SDK where we can (this allows for tagging elements and linking them to kentico). The information from contentTypes.ts is used as a type safe way of providing property names for the smart link data attributes. This has been incredibly nice/convenient. It seems to have one significant downside however, and that is that contentTypes.ts is actually pretty big for our project and the entire thing ends up in our final bundle even if we only use a fraction of it. It's almost the size of angular.

Proposed solution

Right now, contentTypes ends up being a massive single object. If you want metadata about the button content model for instance, you have to pull in the entire object. What would help is if each model object was exported from a single file that contentTypes could then merge together for backwards compatibility. This would allow consumers to import only the model metadata they need and the rest could be tree shaken.

Additional context

Add any other context, screenshots, or reference links about the feature request here.

Simply007 commented 1 year ago

Hello @itworksafisher,

thanks for submitting the issue.

Would you agree that this minor change help you with the tree shaking?

Currently project/contentTypes.ts looks like that.


export const contentTypes = {

  type_codename1: {
    // ...
  },
  type_codename2: {
    // ...
  },
}

So what we can do is to append this export(s) to the module:


export const type_codename1 = contentTypes.type_codename1;
export const type_codename2 = contentTypes.type_codename2;

Possibly this addition could be also configurable.

itworksafisher commented 1 year ago

I don't believe this will help with tree shaking. As I understand it, tree shaking works by determining that code is not referenced anywhere so can be excluded. So if I import type_codename1, there is still a reference to contentTypes. It can't be shaken out.

This is why I suggested splitting up contentTypes in to multiple files. Then I can reference only the contentTypes I need, and the rest can be shaken out.

Creating an alias to a more complex object doesn't remove the more complex object from referencing. I.e. we can't determine the value of type_codename1 without importing the entire contentTypes object.

Simply007 commented 1 year ago

You are correct.

I drafted a solution for generating the content types both ways to ensure the backward compatibility in https://github.com/kontent-ai/model-generator-js/pull/46 - the output sample would look like tkat: https://github.com/kontent-ai/model-generator-js/blob/fix/44/tree-shaking/sample/project/contentTypes.ts

What do you think @itworksafisher?

Simply007 commented 1 year ago

Hello @itworksafisher, this issue has gone quiet. 👻 It’s been a while since the last update here.

If we missed anything on this issue or you want to continue to work on that - feel free to re-open the issue.

If the issue is reopened, @IvanKiral can re-open and finish the draft PR.

Thanks for being a part of the Kontent.ai community!

itworksafisher commented 1 year ago

How do I re-open the issue? Seems I can only create a new one or edit the title. The linked sample looks like it would accomplish what I need.

Enngage commented 1 year ago

Hey @itworksafisher ,

Even though the drafted PR could help in some cases, it's probably not universal as it depends on how many content types you end up using within your app.

I would like to explore different options:

1) Provide the ability to generate full or slim project models. The full would be as it is right now and slim would contain only codenames. In my testing project, it reduced the size of the file by about 75% (of course it depends on the number of elements, names etc.). In my experience, you are probably only using codenames, especially if the code is used within a browser context.

2) Provide strong typing with the use of typescript's Type. This is a bit tricky, but it's doable and it would reduce the size of the file significantly because the code gets stripped from the runtime bundle. I don't yet have any specifics or examples of how it could look like, but there are some possibilities.

At this time I wouldn't want to proceed with the drafted PR due to added complexity and the fact it doesn't really solve the issue (even though it might help in some specific scenarios). But if you really need to use only few content types, then you might as well just delete the types you don't need from the bundle.

For example, in one of my internal apps I have a script that generates the models, but I have a "post-processing" where I remove the stuff I don't need (the external IDs, names..) as well as content types that I don't need to use. All of this is automatic and I only need to whitelist content types which I want to use in the browser context. With this, I'm able to reduce the size by 90+%.

itworksafisher commented 1 year ago

1) Sounds like an optimization - stripping out everything but codename. Could this be as simple as exporting codename literal as a string in the separate file? Could it tree shake the unused object export? Having separate objects already makes a massive impact on our project. We generate about 260 models, but are only referencing about 20. If I could reference those 20 objects directly, without having to import the rest, I'm reducing what I need to include by 90%. Removing non codename elements just takes that a step further, but won't have nearly the same impact, at least in my use case where we generate far more models than we actually use in code. This happens easily if you adopt model generation later in the lifespan of a project, where a heap of content models already exist in kentico.

2) I don't understand how strong typing would fix this. The object literals are strongly typed (unless you are saying each of these metadata objects could have a generalized type they use) since their type is inferred from the object shape. I need a reference to the codename string literal at runtime. If Typescript were going to handle this, it would need to replace the object references with string literals at compile time. Then it could drop all of content types, but that is not a feature I am aware of in Typescript. So if we still need references to these literals at runtime, those content types are still going to have to be included.

itworksafisher commented 1 year ago

@Enngage thoughts on my response?

Enngage commented 1 year ago
  1. Sounds like an optimization - stripping out everything but codename. Could this be as simple as exporting codename literal as a string in the separate file? Could it tree shake the unused object export? Having separate objects already makes a massive impact on our project. We generate about 260 models, but are only referencing about 20. If I could reference those 20 objects directly, without having to import the rest, I'm reducing what I need to include by 90%. Removing non codename elements just takes that a step further, but won't have nearly the same impact, at least in my use case where we generate far more models than we actually use in code. This happens easily if you adopt model generation later in the lifespan of a project, where a heap of content models already exist in kentico.
  2. I don't understand how strong typing would fix this. The object literals are strongly typed (unless you are saying each of these metadata objects could have a generalized type they use) since their type is inferred from the object shape. I need a reference to the codename string literal at runtime. If Typescript were going to handle this, it would need to replace the object references with string literals at compile time. Then it could drop all of content types, but that is not a feature I am aware of in Typescript. So if we still need references to these literals at runtime, those content types are still going to have to be included.

1) No tree-shaking could take place, but the file would be a lot leaner. There might also be a possibility to "limit" the types that are exported. This is probably the way I'll go with the next releases.

2) You could use specific Template literal type (https://www.typescriptlang.org/docs/handbook/2/template-literal-types.html) when you need to reference content type codename/element codename. Of course, if the value would change, you would need to adjust your code to reflect the new value. Typescript would help you "find" the instances where you are using the literal types and prevent you from compiling if you are using invalid values.