DRAFT: Ts migration #640

taylorpwhipp · 2023-01-23T08:55:08Z

This PR is a rough draft/ first step at updating our documentation----> typescript. I will probably need to iterate on this after review since this is my first experience with TS and it touched a lot of files/ code.

…orials

nvie

Hey Taylor, this is a great first draft! I did a review pass for a bunch of updated examples in those MDX files (but not all of them yet). All of those changes look good to me structurally. The comments I left focused mostly on TypeScript best practices, but none of them are anything major and should be easy to fix. Structurally, these changes all look good! 🙌

nvie · 2023-01-23T09:02:36Z

docs/pages/get-started/react-redux.mdx

  cursor: { x: 0, y: 0 },
 };

 const slice = createSlice({
  name: "state",
  initialState,
  reducers: {
-    setCursor: (state, action) => {
+    setCursor: (state, action: PayloadAction<Cursor>) => {


Where is this type PayloadAction defined?

part of the redux-toolkit

We should import it at the top of the code snippet because it’s an addition to the previous step.

nvie · 2023-01-23T09:05:00Z

docs/pages/get-started/react-redux.mdx

+  const others = useSelector<LiveBlocksState, LiveBlocksState["others"]>(
+    (state) => state.liveblocks.others
+  );


Correct! This type annotation is unfortunately still necessary with the current version of @liveblocks/redux. The goal is to eventually not need this and instead let TypeScript be able to derive this type automatically, just like with our React and Zustand packages. But for now, this is indeed the way to type it 👍

nvie · 2023-01-23T09:09:55Z

docs/pages/get-started/react.mdx

 export const {
  suspense: {
    RoomProvider,
    useOthers, // 👈
  },
-} = createRoomContext(client);
+} = createRoomContext< Storage, UserMeta>(client);


These type params need to be in a specific order. All of them are optional, but the order is always fixed. Each of these annotations are valid, but no others:

createRoomContext<Presence>(client); createRoomContext<Presence, Storage>(client); createRoomContext<Presence, Storage, UserMeta>(client); createRoomContext<Presence, Storage, UserMeta, RoomEvent>(client);

Exact, as we just start to introduce useOthers, we need to simply define a new Presence type in this code snippet and let the developer know that explain that a bit bellow.

nvie · 2023-01-23T09:12:08Z

docs/pages/get-started/react.mdx

@@ -337,7 +353,7 @@ function Index() {
        scientist: new LiveObject({
          firstName: "Marie",
          lastName: "Curie",
-        }),
+        }) as Scientist,


Ideally we can avoid the as Scientist cast here and the type def in L341-344 — they should not be necessary. Did you run into issues with this? If so, please lmk, because it may be indicative of another issue?

Agree with @nvie, no cast should be necessary here.

nvie · 2023-01-23T09:18:47Z

docs/pages/tutorials/collaborative-online-whiteboard/react.mdx

  return Math.floor(Math.random() * max);
 }

 function getRandomColor() {
-  return COLORS[getRandomInt(COLORS.length)];
+  return COLORS[getRandomInt(COLORS.length)] as string;


Type casts (using the as operator) are potential hiding places for bugs. I always try to avoid using as and I don't think we should advertise their use in our examples, because they're frowned upon.

Instead, did you perhaps mean:

function getRandomColor(): string { return COLORS[getRandomInt(COLORS.length)]; }

nvie · 2023-01-23T09:20:48Z

docs/pages/tutorials/collaborative-online-whiteboard/react.mdx

  const [{ selectedShape }, setPresence] = useMyPresence();
  const others = useOthers();

  /* ... */

-  const onShapePointerDown = (e, shapeId) => {
+  const onShapePointerDown = (e: any, shapeId: string) => {


Same as with as operator, anys disable TypeScript and as a result they create places where bugs can lurk. For that reason, the use of any is frowned upon and we should not advertise that in our examples.

In this case — and in many other cases — you can replace this any by an unknown, which is safer:

Suggested change

const onShapePointerDown = (e: any, shapeId: string) => {

const onShapePointerDown = (e: unknown, shapeId: string) => {

ah i hadn't seen that, I had seen most examples with events either imply an implicity "any" or use the above
will change this throughout

nvie · 2023-01-23T09:21:18Z

docs/pages/tutorials/collaborative-online-whiteboard/react.mdx

  const { x, y, fill } = shape;

  return (
    <div
-      onPointerDown={(e) => onShapePointerDown(e, id)}
+      onPointerDown={(e:any) => onShapePointerDown(e, id)}


Ditto for this any + all other instances of any in this file.

GuillaumeSalles

Great start on a more tricky task that it seems!

I did a superficial review, I'll do another past tomorrow. There are a few things that are not necessarily obvious and conventions that we need to figure together. I'm going to help you on the small details :)

GuillaumeSalles · 2023-01-31T02:03:05Z

docs/pages/get-started/react-redux.mdx

  cursor: { x: 0, y: 0 },
 };

 const slice = createSlice({
  name: "state",
  initialState,
  reducers: {
-    setCursor: (state, action) => {
+    setCursor: (state, action: PayloadAction<Cursor>) => {


We should import it at the top of the code snippet because it’s an addition to the previous step.

GuillaumeSalles · 2023-01-31T02:06:41Z

docs/pages/get-started/react.mdx

 import { createClient } from "@liveblocks/client";
 import { createRoomContext } from "@liveblocks/react";
+import { Presence, UserMeta, Storage } from "./types";


"./types" does not exist here and we didn't introduce Presence, UserMeta and Storage yet. These should come in a future step.

GuillaumeSalles · 2023-01-31T02:06:52Z

docs/pages/get-started/react.mdx


 const client = createClient({
  publicApiKey: "{{PUBLIC_KEY}}",
 });

+


Extra line to be removed

GuillaumeSalles · 2023-01-31T02:08:14Z

docs/pages/get-started/react.mdx

 export const {
  suspense: {
    RoomProvider,
    useOthers, // 👈
  },
-} = createRoomContext(client);
+} = createRoomContext< Storage, UserMeta>(client);


Exact, as we just start to introduce useOthers, we need to simply define a new Presence type in this code snippet and let the developer know that explain that a bit bellow.

GuillaumeSalles · 2023-01-31T02:09:27Z

docs/pages/get-started/react.mdx

@@ -176,15 +184,16 @@ we call this _presence_.
 We can use _presence_ to hold any object that we wish to share with others. An
 example would be the pixel coordinates of a user’s cursor:

-```js
-cursor: { x: 256, y: 367 }
+```ts


At this step, I would not use any typescript definition.

GuillaumeSalles · 2023-01-31T02:16:11Z

examples/nextjs-live-avatars/components/Avatar.tsx

@@ -12,8 +12,15 @@ import styles from "./Avatar.module.css";
 */

 const IMAGE_SIZE = 48;
+//convert to typescript
+export interface AvatarProps {


Should be renamed to Props because we're in a single file component. This is a common convention.

GuillaumeSalles · 2023-01-31T02:16:33Z

examples/nextjs-live-avatars/components/Avatar.tsx


-export function Avatar({ picture, name }) {
+//convert component to typescript


GuillaumeSalles · 2023-01-31T02:16:49Z

examples/nextjs-live-avatars/pages/_app.tsx

@@ -2,6 +2,7 @@ import { AppProps } from "next/app";
 import Head from "next/head";
 import "../styles/globals.css";

+


GuillaumeSalles · 2023-01-31T02:17:00Z

examples/nextjs-live-cursors-chat/pages/index.tsx

@@ -11,7 +11,7 @@ import Cursor from "../components/Cursor";
 import FlyingReaction from "../components/FlyingReaction";
 import ReactionSelector from "../components/ReactionSelector";
 import useInterval from "../hooks/useInterval";
-
+//// 


GuillaumeSalles · 2023-01-31T02:17:33Z

packages/liveblocks-core/.env

@@ -0,0 +1 @@
+LIVEBLOCKS_PUBLIC_KEY="pk_dev_w56d3kiUtvotjLov9whFziBf"


To remove. It's probably missing from our .gitignore because keys should never be commited.

taylorpwhipp added 7 commits January 5, 2023 09:48

changes for examples

f969f09

lots of changes in flight need to add centralization for types in tut…

388b3a1

…orials

more changes

606bf5e

some redux updates

a9f6884

changes to Avatar file

181163e

lots of updates

25433a9

edits to redux file

4995190

nvie reviewed Jan 23, 2023

View reviewed changes

vercel bot had a problem deploying to Preview – examples-nextjs-3d-builder January 23, 2023 19:05 Failure

vercel bot deployed to Preview – examples-nextjs-live-cursors January 23, 2023 19:05 View deployment

vercel bot had a problem deploying to Preview – examples-javascript-live-cursors January 23, 2023 19:05 Failure

vercel bot deployed to Preview – examples-nextjs-live-avatars January 23, 2023 19:05 View deployment

vercel bot deployed to Preview – examples-nextjs-block-text-editor-advanced January 23, 2023 19:05 View deployment

vercel bot deployed to Preview – examples-javascript-todo-list January 23, 2023 19:06 View deployment

vercel bot deployed to Preview – examples-nextjs-live-cursors-chat January 23, 2023 19:06 View deployment

taylorpwhipp force-pushed the TS-Migration branch from 32e5668 to f737b9d Compare January 23, 2023 20:58

vercel bot deployed to Preview – examples-nextjs-live-cursors January 23, 2023 20:59 View deployment

vercel bot deployed to Preview – examples-nextjs-live-avatars January 23, 2023 20:59 View deployment

vercel bot deployed to Preview – examples-nextjs-block-text-editor-advanced January 23, 2023 20:59 View deployment

vercel bot had a problem deploying to Preview – examples-nextjs-3d-builder January 23, 2023 20:59 Failure

vercel bot deployed to Preview – examples-javascript-todo-list January 23, 2023 20:59 View deployment

vercel bot had a problem deploying to Preview – examples-javascript-live-cursors January 23, 2023 21:00 Failure

vercel bot deployed to Preview – examples-nextjs-live-cursors-chat January 23, 2023 21:00 View deployment

updating 3d builder

14c0a3f

taylorpwhipp force-pushed the TS-Migration branch from f737b9d to 14c0a3f Compare January 26, 2023 18:52

vercel bot deployed to Preview – examples-nextjs-live-cursors January 26, 2023 18:52 View deployment

vercel bot deployed to Preview – examples-nextjs-block-text-editor-advanced January 26, 2023 18:52 View deployment

vercel bot deployed to Preview – examples-nextjs-live-cursors-chat January 26, 2023 18:52 View deployment

vercel bot deployed to Preview – examples-javascript-todo-list January 26, 2023 18:52 View deployment

vercel bot had a problem deploying to Preview – examples-nextjs-3d-builder January 26, 2023 18:53 Failure

vercel bot had a problem deploying to Preview – examples-javascript-live-cursors January 26, 2023 18:53 Failure

vercel bot deployed to Preview – examples-nextjs-live-avatars January 26, 2023 18:53 View deployment

GuillaumeSalles reviewed Jan 31, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT: Ts migration #640

DRAFT: Ts migration #640

taylorpwhipp commented Jan 23, 2023

nvie left a comment

nvie Jan 23, 2023

taylorpwhipp Jan 23, 2023

GuillaumeSalles Jan 31, 2023

nvie Jan 23, 2023

nvie Jan 23, 2023

GuillaumeSalles Jan 31, 2023

nvie Jan 23, 2023

GuillaumeSalles Jan 31, 2023

nvie Jan 23, 2023

nvie Jan 23, 2023

taylorpwhipp Jan 23, 2023

nvie Jan 23, 2023

GuillaumeSalles left a comment

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

GuillaumeSalles Jan 31, 2023

	const onShapePointerDown = (e: any, shapeId: string) => {
	const onShapePointerDown = (e: unknown, shapeId: string) => {


		export function Avatar({ picture, name }) {
		//convert component to typescript

		@@ -2,6 +2,7 @@ import { AppProps } from "next/app";
		import Head from "next/head";
		import "../styles/globals.css";

		@@ -0,0 +1 @@
		LIVEBLOCKS_PUBLIC_KEY="pk_dev_w56d3kiUtvotjLov9whFziBf"

DRAFT: Ts migration #640

Are you sure you want to change the base?

DRAFT: Ts migration #640

Conversation

taylorpwhipp commented Jan 23, 2023

nvie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeSalles left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment