Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document compilation performance #20

Closed
rattrayalex opened this issue Apr 17, 2021 · 7 comments
Closed

Document compilation performance #20

rattrayalex opened this issue Apr 17, 2021 · 7 comments

Comments

@rattrayalex
Copy link

rattrayalex commented Apr 17, 2021

This looks like an amazing project! I'd love to build an app around it.

One thing I'm curious about is how slow it might make my TS compilation times as the application grows. Do you have any real-world data of compilation times in large projects? Are you aware of pathological patterns in js schemas that cause slow compilation? Or do you have a theoretical idea of how slow this could get in certain situations?

Thanks!

@tamasfe
Copy link

tamasfe commented May 4, 2021

We are using this in a small-ish project that currently contains ~2k lines of schemas, and here's my take on this so far:

disclaimer: We use esbuild for actual transpiling/bundling, and only use tsc for type checking.

Most of the time it works as intended, and IDE DX is fine.

However we've already ran into inconsistent recursion issues similar to microsoft/TypeScript#41406 on larger schemas. This would not be an issue if the compiler didn't have a baked-in type recursion limit, but it currently does.

Even worse, this behaviour is very dependent on the compiler version, and simply bumping/reverting even a patch version can create/solve such errors.

In some cases splitting up schemas is possible, however it is not always the case, and there can be a lot of @ts-ignore comments and any or unknown types.

These are the issues we've encountered so far, I would definitely recommend the project if only for the provided runtime-safety from the schemas. However don't go all-out on schemas everywhere and stick to regular typescript types if they never actually leave the boundary of your application.

For the record project contains ~10k typescript LoC, here is a type check run:

yarn tsc --noEmit > /dev/null  4,82s user 0,16s system 214% cpu 2,320 total 

In the IDE there is no sign of performance decrease so far as the language server is very good at incremental checks and caching.

@rattrayalex
Copy link
Author

Thank you @tamasfe ! Sounds like this project is probably great to use for smaller schemas, but maybe a codegen-based tool would work better for larger schemas.

@ThomasAribart
Copy link
Owner

Thanks @tamasfe for the feedback! Indeed for the moment I haven't met any performance issues on my projects and haven't received any complaints so far.

The 'Type instantiation is excessively deep and possibly infinite' is definitely a bummer.But from what I've seen so far, the inferred types are still correct: They are just hard to read and a 'ts-ignore' is required to discard the warning.

@tamasfe Can you share the schemas that are incorrectly parsed ? (You mentioned 'any' and 'unknowns') I'm very interested! Thanks🙏

@tamasfe
Copy link

tamasfe commented May 16, 2021

@ThomasAribart The any | unknown could be a side effect of the @ts-ignore comments, but unfortunately we cannot ignore typescript errors, and afaik there's no way to selectively ignore one kind of error.

The project code is not public, however this oneOf beast is part of our public API:

JSON
{
  "type": "object",
  "required": ["entries"],
  "properties": {
    "requestId": {
      "type": "string",
      "description": "A unique request ID for preventing duplicate requests in case of failures.\n\nIt is an optional security feature and it is the API consumer's responsibility\nto use it.",
      "maxLength": 36
    },
    "entries": {
      "type": "array",
      "description": "A list of new entries.\n\nThe order of the entries are based on timestamps, if multiple entries\nhave the same timestamp, their order is based on the position in the array.\n\nThe earliest timestamp in the new entries must be after the timestamp of the\nlatest already existing entry in the DynamO Pricing system.",
      "items": {
        "oneOf": [
          {
            "title": "Instance Dates Entry",
            "description": "This entry configures the start and the end dates of an instance.\n\nEither value can be `null` if the dates are not known or simply don't exist.",
            "type": "object",
            "required": ["dates", "entryType", "timestamp"],
            "properties": {
              "entryType": {
                "type": "string",
                "enum": ["instanceDates"]
              },
              "dates": {
                "type": "object",
                "title": "Date Range",
                "properties": {
                  "start": {
                    "type": "string",
                    "nullable": true,
                    "format": "date-time"
                  },
                  "end": {
                    "type": "string",
                    "nullable": true,
                    "format": "date-time"
                  }
                }
              },
              "timestamp": {
                "title": "Entry Timestamp",
                "description": "The exact date and time of the creation of the entry.\nThis cannot refer to the future.",
                "type": "string",
                "format": "date-time"
              }
            }
          },
          {
            "type": "object",
            "title": "Item Quantity Entry",
            "description": "This entry alters or configures item quantities.\n\nNote that this won't affect sold items, use the `itemReturn` entry for refunds.",
            "required": ["items", "entryType", "timestamp"],
            "properties": {
              "entryType": {
                "type": "string",
                "enum": ["itemQuantity"]
              },
              "items": {
                "type": "array",
                "title": "Item Identifiers",
                "description": "An array of Item Identifiers",
                "items": {
                  "title": "Item ID",
                  "description": "Every distinct dynamically priced item should have a unique identifier within an instance.\n\nThe Id is up to the user of the API with a few restrictions:\n  - Must consist of alphanumeric characters or hyphens.\n  - Must be at least 1 character long.\n  - Must be at most 36 characters long.\n  \nIt is advised to use memorable and predictive IDs when possible,\nhowever only uniqueness is required and enforced by DynamO Pricing.",
                  "type": "string",
                  "pattern": "^[a-zA-Z0-9-]+$",
                  "minLength": 1,
                  "maxLength": 36
                }
              },
              "timestamp": {
                "title": "Entry Timestamp",
                "description": "The exact date and time of the creation of the entry.\nThis cannot refer to the future.",
                "type": "string",
                "format": "date-time"
              }
            },
            "oneOf": [
              {
                "type": "object",
                "title": "Item Quantity Change",
                "description": "The change in item count.",
                "required": ["change"],
                "properties": {
                  "change": {
                    "oneOf": [
                      {
                        "type": "object",
                        "title": "Relative Change",
                        "description": "Relative change compared to the current remaining item count tracked by DynamO Pricing.\nThe total item count cannot be below zero.",
                        "required": ["relative"],
                        "properties": {
                          "relative": { "type": "integer" }
                        }
                      },
                      {
                        "type": "object",
                        "title": "Total Change",
                        "required": ["total"],
                        "description": "The total amount of items available, cannot be below zero.",
                        "properties": {
                          "total": {
                            "oneOf": [
                              { "type": "integer", "minimum": 0 },
                              {
                                "description": "Infinity",
                                "type": "string",
                                "enum": ["inf"]
                              }
                            ]
                          }
                        }
                      }
                    ]
                  }
                }
              },
              {
                "type": "object",
                "title": "Item Quantity Link",
                "description": "Create a quantity link between the item(s) and the target item.\n\nAfter linking, the quantity of the item(s) will equal the quantity of the target item's.\n\nAny consequent quantity changes from sales will affect all linked items equally.\n\nChanging the quantity of the items is possible after linking, however the total\nquantity cannot exceed the target item's.",
                "required": ["link"],
                "properties": {
                  "link": {
                    "type": "object",
                    "title": "Link Options",
                    "required": ["itemId"],
                    "properties": {
                      "itemId": {
                        "type": "string",
                        "description": "The target item ID to link with."
                      }
                    }
                  }
                }
              }
            ]
          },
          {
            "title": "Item Price Entry",
            "description": "Price configuration for items.",
            "type": "object",
            "required": ["entryType", "timestamp", "items"],
            "properties": {
              "entryType": {
                "type": "string",
                "enum": ["itemPrice"]
              },
              "items": {
                "type": "array",
                "title": "Item Identifiers",
                "description": "An array of Item Identifiers",
                "items": {
                  "title": "Item ID",
                  "description": "Every distinct dynamically priced item should have a unique identifier within an instance.\n\nThe Id is up to the user of the API with a few restrictions:\n  - Must consist of alphanumeric characters or hyphens.\n  - Must be at least 1 character long.\n  - Must be at most 36 characters long.\n  \nIt is advised to use memorable and predictive IDs when possible,\nhowever only uniqueness is required and enforced by DynamO Pricing.",
                  "type": "string",
                  "pattern": "^[a-zA-Z0-9-]+$",
                  "minLength": 1,
                  "maxLength": 36
                }
              },
              "timestamp": {
                "title": "Entry Timestamp",
                "description": "The exact date and time of the creation of the entry.\nThis cannot refer to the future.",
                "type": "string",
                "format": "date-time"
              }
            },
            "anyOf": [
              {
                "type": "object",
                "title": "Allowed Price",
                "description": "Set price constraints.",
                "required": ["allowedPrice"],
                "properties": {
                  "allowedPrice": {
                    "oneOf": [
                      {
                        "type": "object",
                        "title": "Price Range",
                        "description": "Allowed price range.",
                        "properties": {
                          "min": {
                            "type": "number",
                            "description": "The minimum allowed price."
                          },
                          "max": {
                            "type": "number",
                            "description": "The maximum allowed price."
                          }
                        }
                      },
                      {
                        "type": "number",
                        "title": "Constant Price",
                        "description": "A constant price."
                      }
                    ]
                  }
                }
              },
              {
                "type": "object",
                "title": "Default Price",
                "required": ["defaultPrice"],
                "description": "Set the default price of the item, it will be used if no other information is available.",
                "properties": {
                  "defaultPrice": {
                    "type": "number",
                    "description": "The default price of the item."
                  }
                }
              },
              {
                "type": "object",
                "title": "Match Prices",
                "description": "Match the price of an another item.\n\nIf the price of the target item changes, the price of the given items\nwill be updated accordingly.\n\nThe price of the items will still honor the given allowed price constraints (if any).",
                "required": ["match"],
                "properties": {
                  "match": {
                    "type": "object",
                    "title": "Match Options",
                    "required": ["itemId"],
                    "properties": {
                      "itemId": {
                        "type": "string",
                        "description": "The target item ID."
                      }
                    },
                    "oneOf": [
                      {
                        "type": "object",
                        "title": "Price Multiplier",
                        "description": "The item prices will be the target item's price multiplied by this value.",
                        "required": ["multiplier"],
                        "properties": {
                          "multiplier": { "type": "number" }
                        }
                      },
                      {
                        "type": "object",
                        "title": "Price Difference",
                        "description": "The item prices will be the target item's price and this value added to it.",
                        "required": ["difference"],
                        "properties": {
                          "difference": { "type": "number" }
                        }
                      },
                      {
                        "type": "object",
                        "title": "Exact Match",
                        "description": "Exactly match the target item's price.",
                        "required": ["exact"],
                        "properties": {
                          "exact": {
                            "type": "boolean",
                            "enum": [true]
                          }
                        }
                      }
                    ]
                  }
                }
              }
            ]
          },
          {
            "type": "object",
            "title": "Item Relationships Entry",
            "deprecated": true,
            "description": "**This entry type is deprecated, you can define specific relationships in their appropriate entries.**\n\nAn entry that sets up relationships between items.\n\nSubsequent relationship entries with the same target item ID will override each other and\nonly the last relationship configuration entry will be in effect.\n\nRelationships can be cleared by sending an empty relationships object.",
            "required": [
              "entryType",
              "itemId",
              "targetItemId",
              "relationships",
              "timestamp"
            ],
            "properties": {
              "entryType": {
                "type": "string",
                "enum": ["itemRelationships"]
              },
              "itemId": {
                "title": "Item ID",
                "description": "Every distinct dynamically priced item should have a unique identifier within an instance.\n\nThe Id is up to the user of the API with a few restrictions:\n  - Must consist of alphanumeric characters or hyphens.\n  - Must be at least 1 character long.\n  - Must be at most 36 characters long.\n  \nIt is advised to use memorable and predictive IDs when possible,\nhowever only uniqueness is required and enforced by DynamO Pricing.",
                "type": "string",
                "pattern": "^[a-zA-Z0-9-]+$",
                "minLength": 1,
                "maxLength": 36
              },
              "targetItemId": {
                "title": "Item ID",
                "description": "Every distinct dynamically priced item should have a unique identifier within an instance.\n\nThe Id is up to the user of the API with a few restrictions:\n  - Must consist of alphanumeric characters or hyphens.\n  - Must be at least 1 character long.\n  - Must be at most 36 characters long.\n  \nIt is advised to use memorable and predictive IDs when possible,\nhowever only uniqueness is required and enforced by DynamO Pricing.",
                "type": "string",
                "pattern": "^[a-zA-Z0-9-]+$",
                "minLength": 1,
                "maxLength": 36
              },
              "relationships": {
                "type": "object",
                "properties": {
                  "price": {
                    "oneOf": [
                      {
                        "type": "object",
                        "title": "Price Multiplier",
                        "required": ["multiplier"],
                        "properties": {
                          "multiplier": {
                            "description": "The item's price will be the target item's price multiplied by this value.",
                            "type": "number"
                          }
                        }
                      },
                      {
                        "type": "object",
                        "title": "Price Difference",
                        "required": ["difference"],
                        "properties": {
                          "difference": {
                            "description": "The item's price will be the target item's price and this value added to it.",
                            "type": "number"
                          }
                        }
                      }
                    ]
                  },
                  "matchQuantity": {
                    "type": "boolean",
                    "description": "If true, the quantity of the items will be based on the target item's quantity.\nAny following changes in quantity of the items will be applied to the target item as well."
                  }
                }
              },
              "timestamp": {
                "title": "Entry Timestamp",
                "description": "The exact date and time of the creation of the entry.\nThis cannot refer to the future.",
                "type": "string",
                "format": "date-time"
              }
            }
          },
          {
            "type": "object",
            "title": "Item Sale Entry",
            "description": "An entry that describes sale of items.",
            "required": ["count", "items", "entryType", "timestamp"],
            "properties": {
              "entryType": {
                "type": "string",
                "enum": ["itemSale"]
              },
              "count": {
                "description": "The amount of items sold.",
                "type": "integer",
                "minimum": 0
              },
              "price": {
                "description": "The price the item(s) were sold for.\nIf not given, the current price of the items will be assumed.",
                "type": "number"
              },
              "items": {
                "type": "array",
                "title": "Item Identifiers",
                "description": "An array of Item Identifiers",
                "items": {
                  "title": "Item ID",
                  "description": "Every distinct dynamically priced item should have a unique identifier within an instance.\n\nThe Id is up to the user of the API with a few restrictions:\n  - Must consist of alphanumeric characters or hyphens.\n  - Must be at least 1 character long.\n  - Must be at most 36 characters long.\n  \nIt is advised to use memorable and predictive IDs when possible,\nhowever only uniqueness is required and enforced by DynamO Pricing.",
                  "type": "string",
                  "pattern": "^[a-zA-Z0-9-]+$",
                  "minLength": 1,
                  "maxLength": 36
                }
              },
              "timestamp": {
                "title": "Entry Timestamp",
                "description": "The exact date and time of the creation of the entry.\nThis cannot refer to the future.",
                "type": "string",
                "format": "date-time"
              }
            }
          },
          {
            "type": "object",
            "title": "Item Return Entry",
            "description": "An entry that is related to item returns and refunds.\n\nThis entry will decrease the amount of sold items.\n\nThis does not imply that the returned items are available again.",
            "required": ["count", "items", "entryType", "timestamp"],
            "properties": {
              "entryType": {
                "type": "string",
                "enum": ["itemReturn"]
              },
              "count": {
                "description": "The amount of returned items.",
                "type": "integer",
                "minimum": 0
              },
              "returnPrice": {
                "description": "The price the item was returned for.\n\nIf the price matches the selling price, the item was fully refunded, 0 means that the item was not refunded.",
                "type": "number"
              },
              "sellPrice": {
                "description": "The price the item was originally sold for.",
                "type": "number"
              },
              "items": {
                "type": "array",
                "title": "Item Identifiers",
                "description": "An array of Item Identifiers",
                "items": {
                  "title": "Item ID",
                  "description": "Every distinct dynamically priced item should have a unique identifier within an instance.\n\nThe Id is up to the user of the API with a few restrictions:\n  - Must consist of alphanumeric characters or hyphens.\n  - Must be at least 1 character long.\n  - Must be at most 36 characters long.\n  \nIt is advised to use memorable and predictive IDs when possible,\nhowever only uniqueness is required and enforced by DynamO Pricing.",
                  "type": "string",
                  "pattern": "^[a-zA-Z0-9-]+$",
                  "minLength": 1,
                  "maxLength": 36
                }
              },
              "timestamp": {
                "title": "Entry Timestamp",
                "description": "The exact date and time of the creation of the entry.\nThis cannot refer to the future.",
                "type": "string",
                "format": "date-time"
              }
            }
          },
          {
            "title": "Pricing Model Entry",
            "description": "Sets the pricing model of the Instance.\n\nAll price calculations following this entry will use the\nconfigured model.\n\nThe available pricing models can be queried for a project [here](#get-/projects/-projectId-/pricingModels).",
            "type": "object",
            "required": ["entryType", "modelId", "timestamp", "options"],
            "properties": {
              "entryType": {
                "type": "string",
                "enum": ["pricingModel"]
              },
              "modelId": {
                "type": "string",
                "description": "The ID of the model."
              },
              "options": {
                "type": "object",
                "description": "The options for the model.\n\nThis object must always exist even if the model\ndoes not support options. If that is the case, an empty object must be provided.",
                "additionalProperties": true
              },
              "timestamp": {
                "title": "Entry Timestamp",
                "description": "The exact date and time of the creation of the entry.\nThis cannot refer to the future.",
                "type": "string",
                "format": "date-time"
              }
            }
          }
        ]
      }
    }
  }
}

In the code it is made up of separate schema objects.

An interesting feature that surprised me is that due to the fact that entryType is unique, when it works, typescript is smart enough to differentiate objects based on that meaning we can switch/case when processing them.

@ThomasAribart
Copy link
Owner

ThomasAribart commented May 19, 2021

Alright, I've run some tests with your schema. Here are some conclusions:

  • There seems to be no warning if you use Typescript less than v4
  • Above v4, the type instanciation is excessively deep and potentially infinite error arises on large schemas.
  • HOWEVER, types still seem to be correctly inferred 👍
  • The anys that you mention probably come from the fact that you're not using TS with the strict: true option. Can you confirm that it is the case @tamasfe ?
  • The unknowns that you mention probably come from the fact that additional properties are not denied by default in JSON schemas. It's definitely opiniated, but I took the decision to warn users by inserting [x: string]: unknown in objects that don't explicitely deny additional properties, as it does more good than harm. You can remove them by adding additionalProperties: false where needed (beware, it'll have an effect on the runtime validation).

ezgif com-gif-maker

For the moment, I don't think there's any way to discard the TS warning from inside the library. So I'm closing this issue.

Cheers !

@rattrayalex
Copy link
Author

Err, this issue wasn't about a TS warning, it was about documenting how well the library handles large schemas. Could the readme be updated with the learnings here?

@ThomasAribart
Copy link
Owner

Alright I'll add a FAQ section in the docs 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants