Marko support rerendering #7460
Conversation
DylanPiercey
requested review from
aaronmcadam,
alexandrebodin,
alterx,
CodeByAlex,
dangreenisrael,
danielduan,
Gongreg,
igor-dv,
jbovenschen,
kazupon,
leonrodenburg,
thani-sh,
naipath,
ndelangen,
renaudtertrais,
rmevans9,
saponifi3d,
shilman,
stijnkoopal,
theinterned,
thomasbertet,
tmeasday,
usulpro and
z4o4z
as code owners
|
This pull request is automatically deployed with Now. Latest deployment for this branch: https://monorepo-git-fork-dylanpiercey-marko-support-rerendering.storybook.now.sh |
|
I botched this PR, give me a minute to fix 😄 |
DylanPiercey
force-pushed
the
marko-support-rerendering
branch
from
41754ea to
d700abe
Compare
|
Alright, I believe I've fixed everything. Happy to address any feedback. |
ndelangen
removed request for
saponifi3d,
alterx,
leonrodenburg,
thomasbertet,
Gongreg,
stijnkoopal,
dangreenisrael,
jbovenschen,
renaudtertrais,
alexandrebodin,
naipath,
igor-dv,
z4o4z,
rmevans9,
usulpro and
CodeByAlex
| @@ -5,16 +5,14 @@ export default { | |||
| title: 'Addons|Knobs/Hello', | |||
| decorators: [withKnobs], | |||
| parameters: { | |||
| component: Hello, | |||
| template: Hello, | |||
There was a problem hiding this comment.
component param is for addon-docs
There was a problem hiding this comment.
Thanks for the pointer! Could you share with me the docs for that config option? I’m new to using story book and want to make sure that Marko is accommodating as much of the ecosystem as possible 🙂.
There was a problem hiding this comment.
It hasn't made it into the main docs yet (coming soon!), but here's the best resource for now. https://docs.google.com/document/d/1un6YX7xDKEKl5-MVb-egnOYN8dynb5Hf7mq0hipk8JE/edit
There was a problem hiding this comment.
Excellent, thanks. Perhaps it would make sense to use component instead of template as a property for the story fns as well just to be consistent. I’ll update the PR shortly.
| @@ -4,8 +4,11 @@ import Button from '../components/action-button/index.marko'; | |||
| export default { | |||
| title: 'Addons|Actions/Button', | |||
| parameters: { | |||
| component: Button, | |||
| options: { panelPosition: 'right' }, | |||
There was a problem hiding this comment.
component param is for addon-docs
DylanPiercey
force-pushed
the
marko-support-rerendering
branch
from
0008b23 to
3282ddd
Compare
|
@shilman I've added back the I've set it up such that if there is a |
|
@DylanPiercey Is this a breaking change? |
|
@shilman this should not be a breaking change. I added a deprecation message for the old behavior. |
Issue:
Currently the Marko API for stories is to return a render result. This limits the ability for stories to be efficiently rerendered while maintaining state and also locks in users to a specific render API for Marko.
What I did
This PR adds a deprecation warning for the old api of returning the
renderSyncresult of a component, and now accepts an object result with the template to render and the input for the template similar to thevue,angular,svelteand some other implementations.With this change I was able to keep track of the currently rendered component instance and only destroy the existing component if the constructor has been modified. This means that when using an addon like
knobsthe component is efficiently rerendered instead of destroyed. This also means that the entire lifecycle of the component is no longer rerun during updates which I think is more inline with what developers expect.How to test
I am unsure what the best way to test this is, however I have updated all examples I could find.