-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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
[docs] AppBar and Textfield demos in TypeScript #13229
[docs] AppBar and Textfield demos in TypeScript #13229
Conversation
@eps1lon It's an interesting initiative! |
"citation needed" (sorry, couldn't resit 😉), but really?
Thank goodness for search! 😄 And +183 lines for what is, in essence, a 3 line diff? Honestly, if a user can't type the demos after reading the docs, they're never going to be able to type their own code. (I say this as someone whose ts is even worse than their js, so not a criticism, just a blunt fact). |
Always valid. I wanted to push this quick and was a bit lazy. Those are examples that would be addressed by this: #12486, #12946, #12709, #13004, #13218, #13372, #13396 and some gitter conversations.
Are you referring to the changed demo? I thought I addressed those concerns with the Proof of Concept tag and repeating that this is unpolished at the moment. The end goal is to have no diff at all.
Let me rephrase my motivation (also in response to @oliviertassinari ). I think our typescript declarations are not tested well. I wanted to put some work into writing new tests but like with every test I feared that this might result in suites that don't really stress test or don't reflect real world use cases. I believe that the demos are the strongest part of this documentation and they are already used for regression tests so why not also use them to test our declarations. But why just hide those in test files when we can also put them into the docs and showcase how good or bad this library works with typescript. In essence our tests are literally part of the documentation. @oliviertassinari Edit:
|
I'm all in for the scientific approach 🔬. It starts with an intuition, it leads to a quick experimentation and finishes with an analysis of the results. repeat ♻️ . |
Directionally I think this is a great idea, as would be porting the unit tests over to TypeScript (@rosskevin took a stab at this a year or so ago but we never got it over the finish line). The more validation code we can write in TypeScript, the more we can simultaneously validate the implementation as well as the fact that the types correspond to that implementation. |
The current implementation generates for the very simple (concerning types) demo BottomAppBar the same JS from TS. I updated the notes about the babel plugin. https://deploy-preview-13229--material-ui.netlify.com/demos/app-bar/#bottom-app-bar is a preview of the JS/TS switch. Currently there is a global one in the docs app bar. The global switch looks out of place and in the current form might not be useful since we won't have much examples in TS to begin with. It's just a first draft and I'm pretty certain it's not gonna make it. The switch per demo looks good though in my opinion and just need some UX improvements since tooltips won't work with disabled elements. State of the logo vs. code might be confusing. I want to signal that clicking it will toggle between JS and TS but then the JS logo is display when TS code is shown. Probably some toggle button group with colorized logo if activated is better. |
775fa48
to
f6b0001
Compare
60b2061
to
0b8e392
Compare
This is now implementation wise complete. I updated the initial post. It's only enabled for the app bar right now. I would like to port another component demo. @oliviertassinari Could you tell me the most or 2nd most popular demo? |
TextField 🥇, Button 🥈, AppBar 🥉. |
5386445
to
6d9e13d
Compare
@eps1lon Should I review this pull request ? |
6d9e13d
to
d86a071
Compare
@oliviertassinari That would be nice. There are still some issues with codesandbox but I think switching to |
ec63de4
to
4135e72
Compare
4135e72
to
c915752
Compare
commit 2035e6daa1ceba1c78d3e8740af8d5f3066bc898 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon Nov 5 10:16:31 2018 +0100 [docs] Fix undefined raw js commit 9ab3761e49d8682530543f1e6808d46cd955f32a Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon Nov 5 10:07:38 2018 +0100 [docs] Enable textfield ts demos commit 8271f5cff4b3f6fe31b564a3f98ed69d8c09f3d6 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 30 11:58:29 2018 +0100 [docs] Port mui#13428 to typescript commit 8b808886235a3690b01335a5d3a6132209b87483 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Fri Oct 26 17:55:02 2018 +0200 [docs] Add typescript demos for TextField Takeaway: - inputProps, inputComponent is badly typed (needs generics though) - computed property keys support is bad in typescript commit 5ab9c988d259cd039e5b2735a47c1c89c9761eec Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 23 13:36:56 2018 +0200 [docs] Use esModuleInterop import conistently commit c9bca0824c0f81fe114f8976bc10464de94306b2 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 23 13:35:32 2018 +0200 [docs] Fix IE 11 compat issues commit b4dd772ee990dd0f9a042a8721aa7a665ded7fce Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 23 13:33:44 2018 +0200 [docs] Remove dead code commit f0a23cffb7a8e2c4faf5c2b4182919efb74d4bdd Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 23 08:21:07 2018 +0200 [ci] Fix job order Running git diff after yarn build will exit with 1 because the size snapshot was updated. commit 114022bd6d1ac98af330f6e7b57ba03062d120ff Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon Oct 22 21:32:54 2018 +0200 [docs] Disabled stackblitz for TS demos commit 7995d67cd09f08541f388124d535452054518d75 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon Oct 22 17:47:14 2018 +0200 [docs] Add guide about ts demos commit 0c65724f14c4de21ced8ce4e8ff91b2edce3b6d5 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon Oct 22 17:09:39 2018 +0200 [docs] Sync demo changes from 06967ec commit 7a0861d855d15bf281fde271e0028d0b1ba9dbb1 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon Oct 22 15:35:03 2018 +0200 [docs] Allow require default interop for ts demos Seems like codesandbox still has issues with this. Gonna investigate if we can fix this upstream commit c8a143e20735be14f692477fab40cd3d4a3040df Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Mon Oct 22 15:34:10 2018 +0200 [docs] Support types for scoped packages commit 48a425d19a53e77fa97692980e364f503491a91b Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Sat Oct 20 18:02:05 2018 +0200 [babel-plugin-unwrap-createStyles] Fix fixtures being transpiled with workspace config commit 03c1ac9d7575c9dc79e8253578dda60f39e33375 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Fri Oct 19 12:21:59 2018 +0200 [docs] Create ts specific codesandbox - needs babel plugin for synthetic default imports commit cdc393185a823bb684ff34be23f20d4897db8962 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Thu Oct 18 18:39:40 2018 +0200 [docs] Fire analytics event when clicking codeLanguage commit 6865f6488c27c1353cae108410d94a165343414b Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Thu Oct 18 18:15:05 2018 +0200 [docs] Add ts version for all app bar demos - Fixes menu not closing in PrimarySearchAppBar - removes some dead code from undefined classnames commit a189a173a371e785916b8797283798e45d4743d8 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Thu Oct 18 17:12:44 2018 +0200 [ci] Check compiled ts demos are equal to js demos commit 95706fdaaa1b6a56da239250b515a36459c87b8b Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 20:51:02 2018 +0200 [docs] Remove debug config commit a90c76a322104787837c1646fc400e84cdc77e6c Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 20:48:56 2018 +0200 [docs] github link and copy code depending on selected code language commit fafa284f397e66b0075a29823debc1c8dbd6b1fb Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 18:14:49 2018 +0200 [docs] Add feature flag for language switch Enabled on a per-component basis. commit 9ef68446d7e42347d30fc796d34f23f15eaf4405 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 18:10:20 2018 +0200 [docs] Add the ability to mark ts demos as outdated This should be used in-sync with ignored demos in transpilation. commit a7033f203278d6e462a016190790a3609aabded5 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 17:18:10 2018 +0200 [docs] Add the ability to ignore ts demos from transpilation This can be useful to defer syncing them upwards from js. commit e7c19cb1f264bf9ec9b6884650b4e3ad8e6942f8 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 16:22:36 2018 +0200 Fix formatting errors commit ea9a6de935394d0c61eff974a8575165ae0ed1aa Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 10:33:23 2018 +0200 [docs] Disable ts switch if not ts version is available commit 5a58595c26108e8ccd18eacf3b16ae2ed40de0a5 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 09:19:53 2018 +0200 [docs] Remove global code language switch Revisit this if we reach critical mass on ts docs commit aecb12fdf96aa761e543ba284d55ffd11a7970f1 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Wed Oct 17 09:15:52 2018 +0200 [docs] Improve code language switch commit 0a587ec8b0f97dfc05352856f7dec1c081e5275e Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 23:37:40 2018 +0200 Fix CI errors commit 82921d138f21272cef11c040ea7a578584563120 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 23:20:36 2018 +0200 [docs] Add local language switch and add language logos - color needs rework: it looks a little bit out of place, bad contrast commit 49861e54f5b5d2e54cac672c4f5a3377731e6255 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 20:45:25 2018 +0200 [docs] Language switch via redux It's pretty slow at the app level. While it is nice to have ts permanently enabled while browsing docs a switch at demo level might be better suited. commit 6a06a060ad17f44dab0d1cc49996455b37f8a2fc Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 19:14:55 2018 +0200 [docs] Another whitespace fix attempt commit fffed851504e587f2abadd6353318eaab9791fdb Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 18:46:46 2018 +0200 [docs] revert to function declaration propTypes in typescript are to strict commit ccb62823cbbacb5da109eb5275277a4ebf797533 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 18:32:38 2018 +0200 [core] Bump @types/react commit d3ac480d1966485b1c3d22672797110c26a82ef5 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 18:25:06 2018 +0200 [babel-plugin-unwrap-createStyles] Fix lint errors commit 1998640e939bfb57e27c265c2e338ef6707bae5a Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 18:21:45 2018 +0200 [docs] Remove diff in trailing whitespace commit 912c2a8d060ff3de9a70eaefa62c011f2bdbc90d Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 18:20:13 2018 +0200 [docs] Write formatted js from ts demos commit 4dcd51f50252823ee5f40dd9e53bfd6821786d5f Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 17:28:08 2018 +0200 [core] Sync tslint with eslint concerning trailing-whitespace commit 05f3f3d78e333ed740f447a2e20724cc1624c363 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 13:59:47 2018 +0200 [core] docuent babel-plugin vs. ts-transformer commit 691a036456486b4cea411d435392746ad714817d Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Tue Oct 16 11:03:08 2018 +0200 [docs] Add babel plugin to unwrap createStyles calls This essentially strips them from the bundle commit a02ffd64a1defb4298030fd2d162c4a71c65b09a Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Sat Oct 13 16:02:11 2018 +0200 Move ts files next to js files commit 32a301a8eb2d0ed5e3b902313d5895dabcfbca32 Author: Sebastian Silbermann <silbermann.sebastian@gmail.com> Date: Sat Oct 13 14:35:14 2018 +0200 Working example without type checking and bad folder structure
c915752
to
33dfce0
Compare
Ah, sorry about that, didn't mean to derail this PR being merged. The toggle button integration is done, but I ran into a roadblock on the MDv2 re-style. Let me know when you're done iterating, and I'll push it in its current form. |
@mbrookes You can go ahead and push your changes. |
WIP ToggleButtons
when preferred code style not available
@mbrookes Thanks for helping with the language switch UI. PR looks fine to me now. Maybe we can improve the whitespace issue later on but I'd rather not discuss formatting anymore in the day and age of |
} | ||
|
||
return ( | ||
<React.Fragment> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need a fragment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's just an artifact from the previous iteration that returned multiple buttons.
@@ -20,6 +20,8 @@ const styles = theme => ({ | |||
}); | |||
|
|||
class ComposedTextField extends React.Component { | |||
labelRef = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bad flow times are back, adding code that does nothing just to make the type checker happy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Knowing something does nothing, got it.
const reqSource = require.context( | ||
'!raw-loader!../../docs/src/pages/demos/app-bar', | ||
false, | ||
/\.(js|tsx)$/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we be able to run the TypeScript version at some point? Looking at the imports, it seems we are only using the .tsx
file for displaying the source of the demo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe. Not sure if we can just use the typescript preset for js files.
I was talking about whitespace in the design sense. e.g. |
This was targeted at https://github.com/mui-org/material-ui/pull/13229/files#r248695540 |
It's very unpolished at the time but I wanted to get some feedback before deep diving.Feature is only available in the app bar demos and text field demos
Motivation
Implementation
This is an early draft. There are some issues that are not adressed yet@babel/preset-typescript
) to ensure sync between js and ts files[ ] apply code language to stackblitz([Post api] template type for React with typescript(tsx) stackblitz/core#573 UPDATE: Might not matter once CRA gets TS support; UPDATE: stackblitz is ignoring the main field. I added a note to the link that stackblitz is not supported for TS demos. )[ ] wait for No Type Checking on initial load codesandbox/codesandbox-client#1217 to be resolved in order to ensure that the codesandbox is working in typescript.It was explained to me that codesandbox bails out because we have to many.d.ts
files. Proposal: Bundling TS module type definitions microsoft/TypeScript#4433 would resolve this. We might improve this by merging index and the actual declaration but this is not local to this PR.Why babel?
The only runtime difference right now is the usage of createStyles which is just the identity function but it's noise for js users. It would be nice to strip this from production code. Typescript tansformer API is either not documented well or the ts team has no goal in unifying this API (see microsoft/TypeScript#14419) hence we use a babel plugin.
I didn't look into that but it might be a good idea anyway to have a webpack/babel plugin that removes this function. I don't know if babel can strip identity functions or if something like
prepack
is required for that.babel-plugin vs. ts-transformer
Issues
code is unformatted. we need to pipe the output to prettierresolved thanks toUsingbabel-plugin-generator-prettier
retainLines
ingeneratorOpts
inbabel
with some manual regex fixing helps fix most of the problems.[ ]edit: will leave it at that; they are way to strictly typed atmpropTypes
need aany
cast[ ] demos need to be written in js for dev hot reloadingyarn docs:typescript --watch
in parallel toyarn docs:dev
addWe haveoutdatedTS
to mark some demos as not-synced. Can later decide if this means that deploy is blocked, hide the switch or just display the warning (current impl)docs/ignore-ts-demo.json
to suppress CI failures.[ ] write demos only in ts and transpile it when building the docs bundle? (might loose contributors)Maintenance overhead is fine for hooks which requires actual implementation port. Porting from js to ts is for most PRs just c&p./cc @mui-org/core-contributors and especially @pelotom as the go2 ts member