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

Typescript definition from example #4290

Open
aboveyunhai opened this issue May 25, 2021 · 3 comments
Open

Typescript definition from example #4290

aboveyunhai opened this issue May 25, 2021 · 3 comments
Labels

Comments

@aboveyunhai
Copy link

aboveyunhai commented May 25, 2021

Description
There are some Typescript errors from the example if it's under a stricter typescript check environment, I would expect same problem from other areas that use the similar codes.

const Leaf = ({ attributes, children, leaf }) => {
if (leaf.bold) {
children = <strong>{children}</strong>
}
if (leaf.code) {
children = <code>{children}</code>
}

Since type { leaf } is either CustomText or EmptyText, there will be a typescript error that bold does not exist on Emptytext
when you try to do leaf.bold.

A better way to do it without worry about leaf definition and type-safe is
if("bold" in leaf) { }
I can create a PR for it but I am not sure it's suitable.

It's quite confusing that all files are using .tsx but none of them actually use typescript. I understand the slate.js typescript is a bit unusual and require some readings but then why not just make these file pure .js since developer would definitely look at the .tsx to get some typescript ideas.

Expectation
No confused type error appearing.

Environment

  • Slate Version: [e.g. 0.59]
  • Operating System: [e.g. iOS]
  • Browser: [e.g. Chrome, Safari]
  • TypeScript Version: [e.g. 3.9.7 - required only if it's a TypeScript issue]

Context
Add any other context about the problem here. (The fastest way to have an issue fixed is to create a pull request with working, tested code and we'll help merge it. Slate is solving a pretty complex problem, and we can't do it without active contributors, so thank you so much for your help!)

@thesunny
Copy link
Collaborator

@aboveyunhai Thanks for submitting the issue. Yes, please feel free to make a PR.

I had no idea you could do if ("bold" in leaf) to get around the TypeScript warnings. That's awesome!

I think some of the typings were missed during the upgrade to TypeScript. Please feel free to add them in.

@aboveyunhai
Copy link
Author

aboveyunhai commented May 26, 2021

@aboveyunhai Thanks for submitting the issue. Yes, please feel free to make a PR.

I had no idea you could do if ("bold" in leaf) to get around the TypeScript warnings. That's awesome!

I think some of the typings were missed during the upgrade to TypeScript. Please feel free to add them in.

@thesunny I don't think it's a proper solution yet because bold in leaf only check if the property exist on the leaf. It will be problematic and dangerous if you have such implementation that bold: undefined and bold: false.
The only thing I know so far it's to use if ((leaf as FormattedText).bold) .
Assume type CustomText = EmptyText | FormattedText;
This is a 100% guarantee.
Hopefully it wouldn't mislead you. I'm slightly worried if you use the in keyword in some of implementation due to my mis info.

@thesunny
Copy link
Collaborator

thesunny commented May 26, 2021

I guess you could try if ("bold" in leaf && leaf.bold)...

Not sure if that works in TypeScript or not...

I define text like { text: string, bold?: true } so it would work for me but I see that it could be dangerous depending on how you defined your types.

FYI, the solution I'm currently using is to define the unstyled text like { text: string, bold?: undefined } and then using leaf.bold but I find it... uncomfortable. But it doesn't result in TypeScript errors at least. I personally avoid using as anywhere in my TypeScript if possible.

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

No branches or pull requests

2 participants