Skip to content

Commit

Permalink
Merge pull request #18 from threlte/stl-render-core-prep
Browse files Browse the repository at this point in the history
Fix memory leak and potential props clash
  • Loading branch information
michealparks authored Jun 21, 2024
2 parents dd8d735 + 21bdb2b commit d3c23b8
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/pink-kiwis-flash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@threlte/test': patch
---

Fix memory leak and potential props clash
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"license": "MIT",
"version": "0.2.3",
"scripts": {
"all": "npm run check && npm run lint && npm run test && npm run build",
"dev": "vite dev",
"build": "vite build && npm run package",
"preview": "vite preview",
Expand Down
5 changes: 4 additions & 1 deletion src/lib/Container.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
/** @type {typeof import('svelte').SvelteComponent} */
export let component
/** @type {Record<string, unknown> | undefined} */
export let componentProps
/** @type {import('svelte').SvelteComponent | undefined} */
export let ref = undefined
Expand Down Expand Up @@ -65,5 +68,5 @@
</script>

<SceneGraphObject object={threlteContext.scene}>
<svelte:component this={component} bind:this={ref} {...$$restProps} />
<svelte:component this={component} bind:this={ref} {...componentProps} />
</SceneGraphObject>
29 changes: 19 additions & 10 deletions src/lib/pure.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,18 @@ import Container from './Container.svelte'
import { Core } from './core.svelte.js'

/**
* Check if a value is a plain object.
*
* @param {unknown} maybeObj
* @returns {maybeObj is object}
* @returns {maybeObj is Record<string, unknown>}
*/
const isObject = (maybeObj) => {
return typeof maybeObj === 'object' && maybeObj !== null
return typeof maybeObj === 'object' && maybeObj !== null && !Array.isArray(maybeObj)
}

/** @type {Set<HTMLElement>} */
const targetCache = new Set()

/** @type {Set<HTMLCanvasElement>} */
const canvasCache = new Set()

/** @type {Set<Svelte.SvelteComponent>} */
const componentCache = new Set()

Expand All @@ -28,9 +26,13 @@ const componentCache = new Set()
/**
*
* @param {Record<string, unknown>} options
* @returns {Record<string, unknown> & { target?: HTMLElement }}
* @returns {Record<string, unknown> & { target?: HTMLElement, props?: Record<string, unknown> }}
*/
const checkProps = (options) => {
if (!isObject(options)) {
throw new TypeError('Render options must be a plain object')
}

const keys = Object.keys(options)
const isProps = !keys.some((option) => {
return Core.componentOptions.includes(option)
Expand All @@ -43,7 +45,7 @@ const checkProps = (options) => {
})

if (unrecognizedOptions.length > 0) {
throw Error(`
throw new TypeError(`
Unknown options were found [${unrecognizedOptions}]. This might happen if you've mixed
passing in props with Svelte options into the render function. Valid Svelte options
are [${Core.componentOptions}]. You can either change the prop names, or pass in your
Expand All @@ -52,6 +54,10 @@ const checkProps = (options) => {
`)
}

if (options.props && !isObject(options.props)) {
throw new TypeError('`props` option must be a plain object')
}

return options
}

Expand Down Expand Up @@ -95,22 +101,24 @@ export const render = (Component, componentOptions = {}, renderOptions = {}) =>

/** @type {HTMLCanvasElement} */
const canvas = renderOptions.canvas ?? document.createElement('canvas')
canvasCache.add(canvas)

/** @type {any} */
const ComponentConstructor = 'default' in Component ? Component.default : Component

/** @type {any} */
const anyContainer = Container

/** @type {Record<string, unknown> | undefined} */
let componentProps = checkedOptions.props

const component = Core.renderComponent(
anyContainer,
{
...checkedOptions,
props: {
...(isObject(checkedOptions.props) ? checkedOptions.props : {}),
canvas,
component: ComponentConstructor,
componentProps,
userSize: renderOptions.userSize,
},
target,
Expand Down Expand Up @@ -175,7 +183,8 @@ export const render = (Component, componentOptions = {}, renderOptions = {}) =>
* @param {Record<string, unknown>} props
*/
rerender: async (props) => {
Core.updateProps(component, props)
componentProps = { ...componentProps, ...props }
Core.updateProps(component, { componentProps })
await Svelte.tick()
},

Expand Down

0 comments on commit d3c23b8

Please sign in to comment.