Skip to content

chore: add type generation improvements for ts #1100

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

Merged
merged 2 commits into from
Jul 3, 2025
Merged

chore: add type generation improvements for ts #1100

merged 2 commits into from
Jul 3, 2025

Conversation

ChiragAgg5k
Copy link
Member

@ChiragAgg5k ChiragAgg5k commented Jul 3, 2025

What does this PR do?

  • Add ability to generate types to specific file
  • Fix optional attributes using ? instead of | null
  • Fix Models import error
  • Improve formatting and add auto-generated comments

Test Plan

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

yes

@ChiragAgg5k ChiragAgg5k requested review from Copilot and Meldiron July 3, 2025 10:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds improvements to the TypeScript type generation in the Appwrite CLI, including single-file output support, optional property syntax, and import/formatting fixes.

  • Introduce --output as a file path for single-file languages and validate directory vs. file output.
  • Switch optional model attributes from | null to the ? shorthand and update import statements.
  • Enhance templates with auto-generated comments and cleaner enum formatting.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
templates/cli/lib/type-generation/languages/typescript.js.twig Update type rendering: optional ?, import type Models, add regen instructions.
templates/cli/lib/commands/types.js.twig Detect file vs. directory output, support single-file destinations, error on mismatch.
Comments suppressed due to low confidence (2)

templates/cli/lib/commands/types.js.twig:69

  • [nitpick] The alias rawOutputPath for rawOutputDirectory may be confusing; consider renaming the function parameter to rawOutputPath directly or removing the extra alias for clarity.
  const rawOutputPath = rawOutputDirectory;

templates/cli/lib/commands/types.js.twig:71

  • Add unit tests for the new single-file output logic (e.g., when isFileOutput is true and meta.isSingleFile()), to ensure correct path resolution and error handling.
  const isFileOutput = !!outputExt;

outputDirectory = path.dirname(rawOutputPath);
singleFileDestination = rawOutputPath;
} else {
throw new Error("Output path must be a directory for languages that generate multiple files.");
Copy link
Preview

Copilot AI Jul 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message could include the invalid path value to aid debugging, e.g., Invalid output path: ${rawOutputPath}. Directory required for multi-file languages.

Suggested change
throw new Error("Output path must be a directory for languages that generate multiple files.");
throw new Error(`Invalid output path: ${rawOutputPath}. Output path must be a directory for languages that generate multiple files.`);

Copilot uses AI. Check for mistakes.

<% for (const [index, element] of Object.entries(attribute.elements)) { -%>
<%- toUpperSnakeCase(element) %> = "<%- element %>",
<% const entries = Object.entries(attribute.elements); -%>
<% for (let i = 0; i < entries.length; i++) { -%>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too complex for me, sorry. Can you show me what this code looks like generated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the change is just to remove the comma at the end of enum class:

before:

export enum Gender {
  MALE = "male",
  FEMALE = "female",
  NON_BINARY = "non-binary",
}

after:

export enum Gender {
  MALE = "male",
  FEMALE = "female",
  NON_BINARY = "non-binary"
}

@@ -95,7 +110,7 @@ const typesCommand = actionRunner(async (rawOutputDirectory, {language}) => {
getType: meta.getType
});

const destination = path.join(outputDirectory, meta.getFileName());
const destination = singleFileDestination || path.join(outputDirectory, meta.getFileName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do basic QA, make sure this work in scenarios like:

  • Folder name but with a dot (not file)
  • file in nested non-existing folder

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • just a dot generates a normal file:
Screenshot 2025-07-03 at 5 37 00 PM
  • using nested non existing folder creates it:
Screenshot 2025-07-03 at 5 37 42 PM
  • using nested non existing folder + file name creates it as well
Screenshot 2025-07-03 at 5 38 21 PM

@Meldiron Meldiron merged commit 09e87e6 into master Jul 3, 2025
37 checks passed
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

Successfully merging this pull request may close these issues.

2 participants