Skip to content

Commit

Permalink
Add runtime type checks to r3f (theatre-js#323)
Browse files Browse the repository at this point in the history
* Add better error/warning messages to r3f

* Fix notifications playground
  • Loading branch information
AndrewPrifer authored Oct 21, 2022
1 parent dee2361 commit 965d708
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 26 deletions.
4 changes: 2 additions & 2 deletions packages/playground/src/shared/notifications/index.tsx
Original file line number Diff line number Diff line change
@@ -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()
Expand Down
50 changes: 35 additions & 15 deletions packages/r3f/src/main/editable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <Keys extends keyof JSX.IntrinsicElements>(
config: EditableFactoryConfig,
Expand All @@ -33,6 +34,12 @@ const createEditable = <Keys extends keyof JSX.IntrinsicElements>(
: {}) &
RefAttributes<JSX.IntrinsicElements[U]>

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(
(
{
Expand All @@ -45,6 +52,20 @@ const createEditable = <Keys extends keyof JSX.IntrinsicElements>(
}: 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<JSX.IntrinsicElements[U]>()
Expand Down Expand Up @@ -75,25 +96,24 @@ const createEditable = <Keys extends keyof JSX.IntrinsicElements>(
const dreiComponent =
Component.charAt(0).toUpperCase() + Component.slice(1)

console.warn(
`You seem to have declared the camera %c${theatreKey}%c simply as <e.${Component} ... />. 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 <e.${Component} />`,
`You seem to have declared the camera "${theatreKey}" simply as \`<e.${Component} ... />\`. 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<EditableCamera
\`\`\`
<EditableCamera
theatreKey="${theatreKey}"
makeDefault
>`,
'font-style: italic;',
'font-style: inherit;',
'background: black; color: white;',
'background: inherit; color: inherit',
'background: black; color: white;',
>
\`\`\`
`,
)
}
}, [Component, theatreKey])
Expand Down
15 changes: 11 additions & 4 deletions packages/r3f/src/main/editableFactoryConfigUtils.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions theatre/core/src/coreExports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}

/**
Expand Down
32 changes: 29 additions & 3 deletions theatre/shared/src/notify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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.`,
)
})
1 change: 0 additions & 1 deletion theatre/studio/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 4 additions & 1 deletion theatre/studio/src/notify.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ const NotificationMessage = styled.div`
}
.code {
overflow: auto;
font-family: monospace;
background: rgba(0, 0, 0, 0.3);
padding: 4px;
Expand Down Expand Up @@ -175,6 +176,7 @@ const COLORS = {
info: '#3b82f6',
success: '#10b981',
warning: '#f59e0b',
error: '#ef4444',
}

const IndicatorDot = styled.div<{type: NotificationType}>`
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -273,6 +275,7 @@ export const notify: Notifiers = {
warning: createHandler('warning'),
success: createHandler('success'),
info: createHandler('info'),
error: createHandler('error'),
}

//region Styles
Expand Down

0 comments on commit 965d708

Please sign in to comment.