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

fix(sdk): explicit type for project template #1779

Merged
merged 2 commits into from
Apr 2, 2022

Conversation

sean-perkins
Copy link
Contributor

Current Behavior

When working with the public API methods, such as openProject, it is unclear what the available options for the templates are. When executing these functions, Stackblitz will throw an error on the site about an invalid template, but does not inform developers about available template values.

New Behavior

This PR makes the template value of the Project interface have strict types of the available options. This allows developers to know the available template values when working with the public API.

Other Information

It appears the API documentation: https://developer.stackblitz.com/docs/platform/javascript-sdk/#sdkopenprojectproject-opts could also be updated to reference the current latest list.

@fvsch fvsch changed the base branch from main to sdk-1-7 March 31, 2022 19:18
@fvsch fvsch requested review from fvsch and EricSimons March 31, 2022 19:51
@fvsch
Copy link
Contributor

fvsch commented Mar 31, 2022

Thanks a bunch for the PR @sean-perkins.

We considered doing this before but opted not too at the time, because we don't want to block users of old versions of the SDK from using a newly supported template type.

That being said,

  1. It's a nice DX improvement for user of TypeScript or of IDEs that support TS types in JS (e.g. VS Code).
  2. The amount of "blocking" this will do is limited, since:
    • We do have console warnings already for project types that the SDK doesn't know about.
    • JavaScript users won't be affected (no compilations errors blocking a build).
    • TypeScript users can always escape it with template: 'new-template-name' as any.

IMO that's good enough to make the jump and use a more precise type, as per this PR. What do you think @EricSimons?

Copy link
Contributor

@fvsch fvsch left a comment

Choose a reason for hiding this comment

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

Let's get this in, as it'll be released in the 1.7.0-alpha.* channel for now, and we can always go back to a more permissive type if need be before 1.7.0.

@fvsch fvsch merged commit c20d6b9 into stackblitz:sdk-1-7 Apr 2, 2022
@sean-perkins sean-perkins deleted the fix/strict-template-types branch April 14, 2022 02:48
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