Skip to content

Conversation

@fvsch
Copy link
Contributor

@fvsch fvsch commented Mar 10, 2023

When using the SDK's embedProject and openProject methods, which expect a project definition object, our TypeScript types require the description to be provided.

Turns out that the StackBlitz backend doesn't actually require a description.

In my own tests, having to come up with a description for a simple example, test or reproduction is often a chore. I've definitely used description: '' before as a workaround. I think we can get out of our users's way and make the description field optional.

What do you think?

@fvsch fvsch requested review from sulco, sylwiavargas and ykmsd March 10, 2023 18:00
@fvsch fvsch force-pushed the fvsch/make-project-description-optional branch from 5e6fa35 to 8d5e782 Compare March 10, 2023 18:14
Copy link

@sylwiavargas sylwiavargas left a comment

Choose a reason for hiding this comment

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

I think this totally makes sense.

@fvsch fvsch force-pushed the fvsch/make-project-description-optional branch from 8d5e782 to fb61bc9 Compare March 10, 2023 18:32
@fvsch fvsch force-pushed the fvsch/make-project-description-optional branch from fb61bc9 to 4615fe5 Compare March 10, 2023 18:36
export interface Project {
title: string;
description: string;
description?: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🌟 Main type change

form.appendChild(createHiddenInput('project[template]', project.template));
addInput('project[title]', title);
if (typeof description === 'string' && description.length > 0) {
addInput('project[description]', description);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did a bit of refactoring of the createProjectForm function, but the main functional change is here: conditionally creating an input to send the description in the POST request.

<head><title></title></head>
<body>
<form method=\\"POST\\" style=\\"display:none!important;\\" action=\\"https://stackblitz.com/run?embed=1\\" id=\\"sb_run\\"><input type=\\"hidden\\" name=\\"project[title]\\" value=\\"Test Project\\"><input type=\\"hidden\\" name=\\"project[description]\\" value=\\"This is a test\\"><input type=\\"hidden\\" name=\\"project[template]\\" value=\\"javascript\\"><input type=\\"hidden\\" name=\\"project[files][package.json]\\" value=\\"{
<form method=\\"POST\\" style=\\"display:none!important;\\" action=\\"https://stackblitz.com/run?embed=1\\" id=\\"sb_run\\"><input type=\\"hidden\\" name=\\"project[title]\\" value=\\"Test Project\\"><input type=\\"hidden\\" name=\\"project[template]\\" value=\\"javascript\\"><input type=\\"hidden\\" name=\\"project[files][package.json]\\" value=\\"{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those snapshots are a bit hard to read, especially in GitHub diffs, but they're correct as far as I can tell.

// Check input values
expect(value('project[title]')).toBe('My Test Project');
expect(value('project[description]')).toBe(project.description);
expect(value('project[description]')).toBe(undefined);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the description from the default test project.

@fvsch fvsch merged commit b768756 into main Mar 13, 2023
@fvsch fvsch deleted the fvsch/make-project-description-optional branch March 16, 2023 08:30
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.

3 participants