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

Paperbase example Typography element uses href attribute improperly #16969

Closed
2 tasks done
lookfirst opened this issue Aug 12, 2019 · 6 comments · Fixed by #17213
Closed
2 tasks done

Paperbase example Typography element uses href attribute improperly #16969

lookfirst opened this issue Aug 12, 2019 · 6 comments · Fixed by #17213
Labels

Comments

@lookfirst
Copy link
Contributor

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

https://github.com/mui-org/material-ui/blob/master/docs/src/pages/premium-themes/paperbase/Header.js#L65

Typography has an href attribute, which is now invalid.

image

If you used typescript, you'd see this as an error and it would cause a nice build failure.

Expected Behavior 🤔

No error.

Steps to Reproduce 🕹

Use typescript.

Context 🔦

Trying to play with this awesome theme.

Your Environment 🌎

Tech Version
Material-UI master
TypeScript 3.5.3
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 13, 2019

This issue is part of a common theme, for instance: #16846, #16843 or #16487.

@lookfirst
Copy link
Contributor Author

@oliviertassinari Thanks. I think the bigger issue is lack of use of Typescript in order to detect these sorts of issues at build time. I wonder at what point MUI gives in and goes full TS. I've personally been waiting years for the news. ;-) It sure would lower the number of issues filed as well as errors in your own codebase, this is a perfect example of it. It would also make maintaining all of those .d.ts files a thing of the past. POJS (plain old javascript) is dead.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 14, 2019

Check #15984. We have hundreds of TypeScript demo that we run tests against. I think that it's a myth, a codebase written in TypeScript will only marginally improve our types, However, the psychological impact is high and interesting to exploit. The most important change so far has been the introduction of the TypeScript demos.

@lookfirst
Copy link
Contributor Author

This bug wouldn't have happened if paperbase was in typescript. That alone is an improvement and not a myth.

I also see this project as a GREAT example for new developers on how to do things well. What you guys have accomplished over the years is awesome. In that sense, leading by example by using Typescript will only help increase its adoption and therefore (arguably) increase the quality of code that is out there that depends on this project. Much in the same way that paperbase is an example.

Moving to typescript also simplifies things... no more .d.ts files and only one set of demos to document. Less code is better.

@oliviertassinari
Copy link
Member

The problem was fixed in #17213.

@lookfirst
Copy link
Contributor Author

FANTASTIC

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

Successfully merging a pull request may close this issue.

3 participants