Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(playground): checkbox bug #2366

Merged
merged 1 commit into from
Apr 7, 2022
Merged

Conversation

NicholasLYang
Copy link
Contributor

@NicholasLYang NicholasLYang commented Apr 6, 2022

Summary

Fixes bug in playground where changing the typescript checkbox deactivates the jsx one.

Test Plan

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 792a3c9
Status: ✅  Deploy successful!
Preview URL: https://cc6f8bbf.tools-8rn.pages.dev

View logs

@ematipico
Copy link
Contributor

ematipico commented Apr 6, 2022

That is intended. Script snippets don't allow JSX nor Typescript

@MichaReiser
Copy link
Contributor

MichaReiser commented Apr 7, 2022

That is intended. Script snippets don't allow JSX nor Typescript

Why so?

@NicholasLYang can you please add more details to your PR (and title). What bug is this PR fixing? How did you test your change?

@ematipico
Copy link
Contributor

That is intended. Script snippets don't allow JSX nor Typescript

Why so?

A script can't have JSX because it requires a compilation step. TypeScript can't be loaded as a script because it requires a compilation step. Is there something I am missing?

@MichaReiser
Copy link
Contributor

MichaReiser commented Apr 7, 2022

That is intended. Script snippets don't allow JSX nor Typescript

Why so?

A script can't have JSX because it requires a compilation step. TypeScript can't be loaded as a script because it requires a compilation step. Is there something I am missing?

The only difference between SourceType::Script and Module is that

  • Module is in strict mode by default
  • Module allows ES import/export statements.

The Script and Module wording matches the wording of the ECMAScript specification (Script, Module)

@NicholasLYang
Copy link
Contributor Author

If we didn't want script snippets to have TypeScript or JSX, perhaps the better option would be to hide the checkboxes when we're on script mode? Or deactivate them. But I don't see anything wrong with someone trying script + jsx + typescript

Comment on lines -54 to -55
setIsTypeScript(false);
setIsJsx(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this was an oversight of mine. I wanted to apply these two calls when source type changes. Sorry about that

@NicholasLYang NicholasLYang merged commit e8e03c2 into main Apr 7, 2022
@NicholasLYang NicholasLYang deleted the fix/playground-checkbox-logic branch April 7, 2022 15:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants