From 965d7085dcf5b8f876594c71d6d1b4899d5594f8 Mon Sep 17 00:00:00 2001 From: Andrew Prifer <2991360+AndrewPrifer@users.noreply.github.com> Date: Fri, 21 Oct 2022 21:17:45 +0200 Subject: [PATCH] Add runtime type checks to r3f (#323) * Add better error/warning messages to r3f * Fix notifications playground --- .../src/shared/notifications/index.tsx | 4 +- packages/r3f/src/main/editable.tsx | 50 +++++++++++++------ .../src/main/editableFactoryConfigUtils.ts | 15 ++++-- theatre/core/src/coreExports.ts | 1 + theatre/shared/src/notify.ts | 32 ++++++++++-- theatre/studio/src/index.ts | 1 - theatre/studio/src/notify.tsx | 5 +- 7 files changed, 82 insertions(+), 26 deletions(-) diff --git a/packages/playground/src/shared/notifications/index.tsx b/packages/playground/src/shared/notifications/index.tsx index 40f65f4..00bd26f 100644 --- a/packages/playground/src/shared/notifications/index.tsx +++ b/packages/playground/src/shared/notifications/index.tsx @@ -1,7 +1,7 @@ import React from 'react' import ReactDOM from 'react-dom' -import studio, {notify} from '@theatre/studio' -import {getProject} from '@theatre/core' +import studio from '@theatre/studio' +import {getProject, notify} from '@theatre/core' import {Scene} from './Scene' studio.initialize() diff --git a/packages/r3f/src/main/editable.tsx b/packages/r3f/src/main/editable.tsx index 87476c6..9b52f17 100644 --- a/packages/r3f/src/main/editable.tsx +++ b/packages/r3f/src/main/editable.tsx @@ -10,6 +10,7 @@ import type {EditableFactoryConfig} from './editableFactoryConfigUtils' import {makeStoreKey} from './utils' import type {$FixMe, $IntentionalAny} from '../types' import type {ISheetObject} from '@theatre/core' +import {notify} from '@theatre/core' const createEditable = ( config: EditableFactoryConfig, @@ -33,6 +34,12 @@ const createEditable = ( : {}) & RefAttributes + if (Component !== 'primitive' && !type) { + throw new Error( + `You must provide the type of the component out of which you're creating an editable. For example: editable(MyComponent, 'mesh').`, + ) + } + return forwardRef( ( { @@ -45,6 +52,20 @@ const createEditable = ( }: Props, ref, ) => { + //region Runtime type checks + if (typeof theatreKey !== 'string') { + throw new Error( + `No valid theatreKey was provided to the editable component. theatreKey must be a string. Received: ${theatreKey}`, + ) + } + + if (Component === 'primitive' && !editableType) { + throw new Error( + `When using the primitive component, you must provide the editableType prop. Received: ${editableType}`, + ) + } + //endregion + const actualType = type ?? editableType const objectRef = useRef() @@ -75,25 +96,24 @@ const createEditable = ( const dreiComponent = Component.charAt(0).toUpperCase() + Component.slice(1) - console.warn( - `You seem to have declared the camera %c${theatreKey}%c simply as . This alone won't make r3f use it for rendering. - -The easiest way to create a custom animatable ${dreiComponent} is to import it from @react-three/drei, and make it editable. - -%cimport {${dreiComponent}} from '@react-three/drei' -const EditableCamera = editable(${dreiComponent}, '${Component}')%c + notify.warning( + `Possibly incorrect use of `, + `You seem to have declared the camera "${theatreKey}" simply as \`\`. This alone won't make r3f use it for rendering. +The easiest way to create a custom animatable \`${dreiComponent}\` is to import it from \`@react-three/drei\`, and make it editable. +\`\`\` +import {${dreiComponent}} from '@react-three/drei' +const EditableCamera = + editable(${dreiComponent}, '${Component}') +\`\`\` Then you can use it in your JSX like any other editable component. Note the makeDefault prop exposed by drei, which makes r3f use it for rendering. - -%c`, - 'font-style: italic;', - 'font-style: inherit;', - 'background: black; color: white;', - 'background: inherit; color: inherit', - 'background: black; color: white;', +> +\`\`\` +`, ) } }, [Component, theatreKey]) diff --git a/packages/r3f/src/main/editableFactoryConfigUtils.ts b/packages/r3f/src/main/editableFactoryConfigUtils.ts index 48962e6..e16064d 100644 --- a/packages/r3f/src/main/editableFactoryConfigUtils.ts +++ b/packages/r3f/src/main/editableFactoryConfigUtils.ts @@ -1,4 +1,5 @@ import type {UnknownShorthandCompoundProps} from '@theatre/core' +import {notify} from '@theatre/core' import {types} from '@theatre/core' import type {Object3D} from 'three' import type {IconID} from '../extension/icons' @@ -77,11 +78,17 @@ export const createVectorPropConfig = ( z: propValue.z, } : // show a warning and return defaultValue - (console.warn( - `Couldn't parse prop %c${key}={${JSON.stringify( + (notify.warning( + `Invalid value for vector prop "${key}"`, + `Couldn't make sense of \`${key}={${JSON.stringify( propValue, - )}}%c, falling back to default value.`, - 'background: black; color: white', + )}}\`, falling back to \`${key}={${JSON.stringify([ + defaultValue.x, + defaultValue.y, + defaultValue.z, + ])}}\`. + +To fix this, make sure the prop is set to either a number, an array of numbers, or a three.js Vector3 object.`, ), defaultValue) ;(['x', 'y', 'z'] as const).forEach((axis) => { diff --git a/theatre/core/src/coreExports.ts b/theatre/core/src/coreExports.ts index e30d73d..b652689 100644 --- a/theatre/core/src/coreExports.ts +++ b/theatre/core/src/coreExports.ts @@ -15,6 +15,7 @@ import type {$IntentionalAny, VoidFn} from '@theatre/shared/utils/types' import coreTicker from './coreTicker' import type {ProjectId} from '@theatre/shared/utils/ids' import {_coreLogger} from './_coreLogger' +export {notify} from '@theatre/shared/notify' export {types} /** diff --git a/theatre/shared/src/notify.ts b/theatre/shared/src/notify.ts index 58f5ae4..db698bb 100644 --- a/theatre/shared/src/notify.ts +++ b/theatre/shared/src/notify.ts @@ -2,7 +2,7 @@ import logger from './logger' import * as globalVariableNames from './globalVariableNames' export type Notification = {title: string; message: string} -export type NotificationType = 'info' | 'success' | 'warning' +export type NotificationType = 'info' | 'success' | 'warning' | 'error' export type Notify = ( /** * The title of the notification. @@ -37,13 +37,31 @@ export type Notifiers = { * Show an info notification. */ info: Notify + /** + * Show an error notification. + */ + error: Notify } const createHandler = (type: NotificationType): Notify => (...args) => { - if (type === 'warning') { - logger.warn(args[1]) + switch (type) { + case 'success': { + logger.debug(args.slice(0, 2).join('\n')) + break + } + case 'info': { + logger.debug(args.slice(0, 2).join('\n')) + break + } + case 'warning': { + logger.warn(args.slice(0, 2).join('\n')) + break + } + case 'error': { + // don't log errors, they're already logged by the browser + } } // @ts-ignore @@ -54,4 +72,12 @@ export const notify: Notifiers = { warning: createHandler('warning'), success: createHandler('success'), info: createHandler('info'), + error: createHandler('error'), } + +window?.addEventListener('error', (e) => { + notify.error( + `An error occurred`, + `${e.message}\n\nSee **console** for details.`, + ) +}) diff --git a/theatre/studio/src/index.ts b/theatre/studio/src/index.ts index b54a554..1464e51 100644 --- a/theatre/studio/src/index.ts +++ b/theatre/studio/src/index.ts @@ -78,7 +78,6 @@ import {notify} from '@theatre/studio/notify' window[globalVariableNames.notifications] = { notify, } -export {notify} export type {IScrub} from '@theatre/studio/Scrub' export type { diff --git a/theatre/studio/src/notify.tsx b/theatre/studio/src/notify.tsx index 502497d..1679602 100644 --- a/theatre/studio/src/notify.tsx +++ b/theatre/studio/src/notify.tsx @@ -135,6 +135,7 @@ const NotificationMessage = styled.div` } .code { + overflow: auto; font-family: monospace; background: rgba(0, 0, 0, 0.3); padding: 4px; @@ -175,6 +176,7 @@ const COLORS = { info: '#3b82f6', success: '#10b981', warning: '#f59e0b', + error: '#ef4444', } const IndicatorDot = styled.div<{type: NotificationType}>` @@ -214,7 +216,7 @@ const massageMessage = (message: string) => { * Creates handlers for different types of notifications. */ const createHandler = - (type: 'warning' | 'success' | 'info'): Notify => + (type: NotificationType): Notify => (title, message, docs = [], allowDuplicates = false) => { // We can disallow duplicates. We do this through checking the notification contents // against a registry of already displayed notifications. @@ -273,6 +275,7 @@ export const notify: Notifiers = { warning: createHandler('warning'), success: createHandler('success'), info: createHandler('info'), + error: createHandler('error'), } //region Styles