Existing Effects & Refs #196
Conversation
| @@ -13,6 +13,6 @@ export default (component: RootTramOneComponent, container: Container) => { | |||
|
|
|||
| // this sadly needs to be wrapped in some element so we can process effects | |||
| // otherwise the root node will not have effects applied on it | |||
| const renderedApp = html`<div><app /></div>`; | |||
| const renderedApp = html`<tram-one><app /></tram-one>`; | |||
There was a problem hiding this comment.
In the UI, this appears like the following:
This is better than <div>, because we won't inherit any unexpected styles or formatting.
| @@ -17,7 +17,7 @@ import { UrlMatchResults } from './types'; | |||
| * | |||
| * @returns object with a `matches` key, and (if it matched) path and query parameters | |||
| */ | |||
| export default (pattern: string): UrlMatchResults => { | |||
| export default (pattern?: string): UrlMatchResults => { | |||
There was a problem hiding this comment.
this should have always been optional! This better matches the interface of the underlying library.
| @@ -55,7 +55,7 @@ const processTramTags = (node: Node | TramOneElement) => { | |||
| const effectReaction = observe(() => { | |||
| // verify that cleanup is a function before calling it (in case it was a promise) | |||
| if (typeof cleanup === 'function') cleanup(); | |||
| cleanup = effect(); | |||
| cleanup = effect(node); | |||
There was a problem hiding this comment.
this is what is required for allowing us to accept ref as a parameter for useEffect - we pass the node we are processing back into the effect it defined!
| // cleanup effects will be populated when new effects are processed | ||
| tagResult[TRAM_TAG_CLEANUP_EFFECTS] = []; |
There was a problem hiding this comment.
there's no reason this should have been populated by the existing new effects before...
| export type TramOneHTMLElement = TramOneElement & HTMLElement; | ||
| export type TramOneSVGElement = TramOneElement & SVGElement; | ||
|
|
There was a problem hiding this comment.
new type definitions, because TramOneElement is too generic, and in reality (right now) it's always an HTMLElement or SVGElement.
If we ever expose registerDOM, people should be able to make their own TramOneElement definitions (which shouldn't be too hard to do).
Summary
This PR makes updates to how
useEffectworks, mostly in prep for #173Fixes #130
Fixes #166
Fixes #112
Changes
refparameter touseEffect.Additional Changes
<tram-one>rather than<div>, this is more clear and obvious, and won't cause formatting issues (since<tram-one>should be undecorated).useUrlParamsoptional (the underlying library has a default value here, so no parameter should be required).refparameter inuseEffectmuch easier (and not require wrapping in a cast toas unknown).Version Bump: Minor
This PR adds new functionality, that should not be breaking in any previously declared way. It's not required by any means for existing projects, so minor feels like the most appropriate here.
Checklist