From 9951b9b1f326248ca251a5587ea5e4e29ee6b598 Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Thu, 27 Jun 2024 15:40:58 -0700 Subject: [PATCH 1/2] Add troubleshooting features for components - In development, warn if a component's init() is improperly an async or Promise-returning function - Eagerly throw error if registering a singleton component that extends Component, instead of at render-time throwing an obscure "Uncaught TypeError: Cannot read properties of undefined (reading 'controller')" - Add troubleshooting section for error when using local model paths in singleton components - Add tests for singleton components --- docs/components/lifecycle.md | 5 ++- docs/guides/troubleshooting.md | 14 ++++++++ src/components.ts | 32 ++++++++++++++--- test/dom/components.mocha.js | 63 ++++++++++++++++++++++++++++++++++ 4 files changed, 108 insertions(+), 6 deletions(-) diff --git a/docs/components/lifecycle.md b/docs/components/lifecycle.md index 2e9c9906..68221715 100644 --- a/docs/components/lifecycle.md +++ b/docs/components/lifecycle.md @@ -99,7 +99,7 @@ Creating a model per component, binding component attributes, and cleaning up co In this case, it is best to declare the component as a "singleton" component. A singleton component is also implemented with a JavaScript class for a controller, but Derby will only instantiate the class once and reuse the same instance of the class each time the component's view is rendered. Derby will not create a model or other properties on the controller, since its instance can be used in multiple places simultaneously. In addition, rendering a singleton component does not invoke `init()`, `create()`, or `destroy()`. -Since singleton components do not have a model, only attribute paths may be used in views. Singleton controllers should consist of only pure functions. +Since singleton components do not have a model, only attribute paths prefixed with `@` may be used in views. Singleton controllers should consist of only pure functions. When a component is used many times on a page, such as a repeated item in a list or a commonly used UI element, it is best to write it statelessly for better performance. View partials are the most lightweight, singleton components allow use of custom JavaScript, and full components have their own model state. @@ -108,6 +108,9 @@ When a component is used many times on a page, such as a repeated item in a list
{{getInitials(@user.fullName)}}
+ + + ``` ```js diff --git a/docs/guides/troubleshooting.md b/docs/guides/troubleshooting.md index e5e4b66b..b48ff491 100644 --- a/docs/guides/troubleshooting.md +++ b/docs/guides/troubleshooting.md @@ -67,3 +67,17 @@ Here are a few common possibilities: * sorting lists on in `init()` might cause the output to be non-deterministic (like alphabetizing / capitalization). Basically a data "bug" would end-up generated different HTML. * putting links in links, which has undefined behavior in HTML * inserting a conditional `
` such as `{{if thisIsTrue}}
stuff
{{/if}}` without restarting the server + +## Error when attempting to use local model paths in singleton components + +A [singleton component](../components/lifecycle#singleton-stateless-components) does not have a local model, so trying to use a local model path like `{{value}}` in its view will fail with this error: + +``` +TypeError: Cannot read properties of undefined (reading 'data') + at PathExpression.get + ... +``` + +To resolve the issue, bind the data via an attribute and refer to it with an attribute path `{{@value}}`. See the linked singleton component documentation for an example. + +Alternatively, if you don't need component controller functions, switch to using a plain [view partial](../components/view-partials) instead. diff --git a/src/components.ts b/src/components.ts index 1b4c7970..2abb46c4 100644 --- a/src/components.ts +++ b/src/components.ts @@ -475,7 +475,19 @@ export class ComponentFactory { // play nice with how CoffeeScript extends class constructors emitInitHooks(context, component); component.emit('init', component); - if (component.init) component.init(component.model); + if (component.init) { + if (util.isProduction) { + component.init(component.model); + } else { + const initReturn: unknown = component.init(component.model); + if (initReturn instanceof Promise) { + console.warn( + `Component ${component.constructor.name} init() should not be an async function:`, + component.init + ); + } + } + } return component.context; } @@ -492,7 +504,7 @@ export class ComponentFactory { function noop() {} -class SingletonComponentFactory{ +class SingletonComponentFactory { constructorFn: SingletonComponentConstructor; isSingleton: true; component: Component; @@ -569,8 +581,18 @@ const _extendComponent = (Object.setPrototypeOf && Object.getPrototypeOf) ? }; export function extendComponent(constructor: SingletonComponentConstructor | ComponentConstructor) { - // Don't do anything if the constructor already extends Component - if (constructor.prototype instanceof Component) return; - // Otherwise, append Component.prototype to constructor's prototype chain + if (constructor.singleton) { + if (constructor.prototype instanceof Component) { + throw new Error('Singleton compoment must not extend the Component class'); + } else { + return; + } + } + // Normal components' constructors must extend Component. + if (constructor.prototype instanceof Component) { + return; + } + // For backwards compatibility, if a normal component doesn't already extend Component, + // then append Component.prototype to the constructor's prototype chain _extendComponent(constructor); } diff --git a/test/dom/components.mocha.js b/test/dom/components.mocha.js index 746553ed..1d9adcca 100644 --- a/test/dom/components.mocha.js +++ b/test/dom/components.mocha.js @@ -1,5 +1,6 @@ var expect = require('chai').expect; var pathLib = require('node:path'); +const { Component } = require('../../src/components'); var domTestRunner = require('../../src/test-utils/domTestRunner'); describe('components', function() { @@ -54,5 +55,67 @@ describe('components', function() { }); }); }); + + it('throws error if registering a singleton component that extends Component', () => { + const harness = runner.createHarness(); + + class MySingletonComponent extends Component { + } + MySingletonComponent.view = { + is: 'my-singleton-component', + source: '
My singleton
' + }; + MySingletonComponent.singleton = true; + expect(() => { + harness.app.component(MySingletonComponent); + }).to.throw(Error, 'Singleton compoment must not extend the Component class'); + }); + }); + + describe('singleton components', () => { + it('do not have their own model when rendered', () => { + const harness = runner.createHarness(); + + class MySingletonComponent { + } + MySingletonComponent.view = { + is: 'my-singleton-component', + source: '
{{@greeting}}
' + }; + MySingletonComponent.singleton = true; + harness.app.component(MySingletonComponent); + + harness.setup(''); + harness.model.set('_page.greeting', 'Hello'); + + const renderResult = harness.renderHtml(); + expect(renderResult.html).to.equal('
Hello
'); + // No Component instance created for singleton components + expect(renderResult.component).to.equal(undefined); + // Singleton components don't get a model allocated under '$components.' like + // normal components would. + expect(harness.model.get('$components')).to.equal(undefined); + }); + + it('can call view functions defined on the component class', () => { + const harness = runner.createHarness(); + + class MySingletonComponent { + emphasize(text) { + return text ? text.toUpperCase() : ''; + } + } + MySingletonComponent.view = { + is: 'my-singleton-component', + source: '
{{emphasize(@greeting)}}
' + }; + MySingletonComponent.singleton = true; + harness.app.component(MySingletonComponent); + + harness.setup(''); + + const renderResult = harness.renderHtml(); + expect(renderResult.html).to.equal('
HELLO
'); + }); }); }); From e9b52bee57579b16441f8b14d18be528bd2a823e Mon Sep 17 00:00:00 2001 From: Eric Hwang Date: Thu, 27 Jun 2024 15:41:54 -0700 Subject: [PATCH 2/2] Add troubleshooting sections for errors "Mutation on uncreated remote document" and "Operation invalid in projected collection" --- docs/guides/troubleshooting.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/guides/troubleshooting.md b/docs/guides/troubleshooting.md index b48ff491..7de02731 100644 --- a/docs/guides/troubleshooting.md +++ b/docs/guides/troubleshooting.md @@ -81,3 +81,22 @@ TypeError: Cannot read properties of undefined (reading 'data') To resolve the issue, bind the data via an attribute and refer to it with an attribute path `{{@value}}`. See the linked singleton component documentation for an example. Alternatively, if you don't need component controller functions, switch to using a plain [view partial](../components/view-partials) instead. + +## Mutation on uncreated remote document + +To perform mutations on a DB-backed document, it must first be loaded in the model. If not, an error `Error: Mutation on uncreated remote document` will be thrown. + +There are a few ways to load a document into the model: +- [Fetching or subscribing to the document](../models/backends#loading-data-into-a-model), either directly via doc id or via a query +- Creating a new document, e.g. via `model.add()` + +When a document is loaded with a [projection](https://share.github.io/sharedb/projections), the mutation must be done using the same projection name. +- For example, if a doc was loaded only with a projection name `model.fetch('notes_title.note-12')`, then mutations must be done with the projection name, `model.set('notes_title.note-12.title', 'Intro')`. +- Trying to mutate using the base collection name in that case, `model.set('notes.note-12.title')`, will result in the "Mutation on uncreated remote document" error. +- If a doc is loaded both with the base collection name and with projections, then mutations can be done with any collection or projection name the doc was loaded with. + +## Invalid op submitted. Operation invalid in projected collection + +Make sure the field being mutated is one of the fields defined in the [projection](https://share.github.io/sharedb/projections). + +If that's not feasible, then fetch/subscribe the doc using its base collection name and do the mutation using the base collection.