sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.75k stars 4.23k forks source link

Shared logic between components #779

Closed stalkerg closed 5 years ago

stalkerg commented 7 years ago

I have several components with differents templates but with some common logic part. Today I can do like this:

import MySharedLogic;

export default {
  oncreate() {
    MySharedLogic.init(this);
  },
}

but if I want to share oncreate, destroy, methods, some part of data it's not possible. I suggest doing something like this:

import MySharedLogic;

export default extend MySharedLogic {
  ...
}

it's should be working as extend for classes in ES6. If it's complicated we can use just Object.assign() to build end object (or same simple logic, without "super" and etc). Anyway, I want to find the solution for good component composition.

Rich-Harris commented 7 years ago

It's possible to share all those things between components 😀

https://svelte.technology/repl?version=1.30.0&gist=b93a144064281af8bb798a2fa1d0622c

The trouble with inheritance mechanisms is that they're inherently harder to read, both for humans and computers. For humans, it means that in order to understand what a particular component is capable of, I have to be familiar with the component itself and MySharedLogic, and whatever MySharedLogic extends from. Used well, it can reduce boilerplate, but it's also a footgun that can result in some truly confusing code.

For computers, it's much worse. The compiler now can't even begin to analyse the component until it's resolved and compiled the entire inheritance chain. Svelte is able to generate efficient code precisely because of the constraints placed on how components are written.

stalkerg commented 7 years ago

https://svelte.technology/repl?version=1.30.0&gist=b93a144064281af8bb798a2fa1d0622c

it's work (convenient) if you have 2-3 methods only.

I don't like extends and strong inherits too, but we should find the good solution for flexible shared logic without tons of code. (not for toy examples)

m59peacemaker commented 7 years ago

Related: https://github.com/sveltejs/svelte/issues/550 https://github.com/sveltejs/svelte/issues/777

Kiho commented 7 years ago

How about this? https://svelte.technology/repl?version=1.30.0&gist=277af0eab460a6104c26d01e804c4903 I got compiler warning but it works like the other sample. And I have big example working in similar way here https://github.com/Kiho/svelte-datatable/blob/master/src/DataTable.html#L158 https://github.com/Kiho/svelte-datatable/blob/master/src/server.js#L27

Rich-Harris commented 6 years ago

This is an ongoing conversation (see also #1041), and I think it would be helpful to gather some realworld situations where sharing things between components is cumbersome, so we can start to figure out the best solutions. I'll open it with this one — I often need to know the width and height of an element, e.g. for doing dataviz scales:

<:Window on:resize='handleResize()' />

<div ref:container><!-- stuff --></div>

<script>
  export default {
    oncreate() {
      this.handleResize();
    },

    methods: {
      handleResize() {
        const { top, right, bottom, left } = this.refs.container.getBoundingClientRect();
        this.set({
          width: right - left,
          height: bottom - top
        });
      }
    }
  };
</script>

That's something that could conceivably be done with a mixin, or perhaps a wrapper component. Though it's not a great example, as it would be more ideal to use bindings that don't exist yet:

<!-- equivalent to the entirety of the above -->
<div bind:width bind:height><!-- stuff --></div>

CC @tomcon who has some other suggestions.

stalkerg commented 6 years ago

After reading #1041 I think we can make preprocessor like a component level mixin. In my case, I have similar widgets but with some extra logic. It's around 10 methods without "compute" or something important for compiler dependency check.

futuraprime commented 6 years ago

There are a lot of cases where a simple binding would do, but there are quite a lot where you want something more complex. For example, @Rich-Harris, you've noted that it's often convenient to get width/height for scales, but you could easily imagine a mixin that transforms d3 scales to account for their ranges. These functions could get quite complicated:

computed: {
  xScaleRanged: (width, xScale, margin) = xScale.range([margin.left, width - margin.right]),
  yScaleRanged: (height, yScale, margin) => yScale.height([height - margin.bottom, margin.top]),
}

A real version of this would probably account for axis positioning too.

TehShrike commented 6 years ago

I want to get to where I have this composition ability:

<IsInViewport>
    <LogClick>
        <Link href="/wherever" />
    </LogClick>
</IsInViewport>

Implemented something like this:

<!-- IsInViewport -->
<span ref:container>
    <slot :isInViewport spread:data />
</span>

<script>
import detectViewportStatusChangedSomehow from 'magic'
export default {
    oncreate() {
        const removeListener = detectViewportStatusChangedSomehow(this.refs.container, isInViewport => {
            this.set({
                isInViewport
            })
        })
        this.on('destroy', removeListener)
    }
}
</script>
<!-- LogClick -->
<span on:click="onclick(event)">
    <slot data-clicks-are-being-logged={{true}} spread:data />
</span>

<script>
export default {
    methods: {
        onclick() {
            console.log('totes clicked')
        }
    }
}
</script>
<!-- Link -->
<a href={{href}} data-is-in-viewport={{isInViewport}} spread:data>
    <slot />
</a>

Related: #195

TehShrike commented 6 years ago

Vue taking a swing at this kind of problem: https://adamwathan.me/renderless-components-in-vuejs/

Rich-Harris commented 6 years ago

Not gonna lie, the slot-scope thing freaks me out. The mental diagram of data flow ends up like a cat's cradle - this is exactly the sort of thing Reactivists point to when they make fun of template languages reinventing JavaScript constructs. And I think that if you're going to do that sort of thing you should steer clear of <slot> (which already has well defined platform behavior) altogether, lest you end up in the uncanny valley.

PaulMaly commented 6 years ago

@TehShrike @Rich-Harris

I believe this is a very common example why the classical approach - separation of behavior and presentation, is more powerful and expansible, than modern approaches like SFC or another "all-in-bunch" things. That's why I never use SFC in Ractive and Vue and sometimes have problems with it in Svelte.

It always confused me, why people at first creates the problems to themselves and then solves these problems in the unevident and freaky ways. Proposed solution with "scoped slots" is a trample of own shit (sorry for my french). This is how it could work with single behavior, multiple representations and no hacks and tricks: example

stalkerg commented 6 years ago

classical approach - separation of behavior and presentation

In my case, I should just share some common logic in behavior and not in presentation. In my original problem, I have many charts components with ChartJS integration and I want to share all this integration logic between my components (LineChart, PieChart, PointChart and etc). MySharedLogic.init(this) working fine but architecture became more unpredictable.

PaulMaly commented 6 years ago

@stalkerg yep, I understand. My reply was addressed to "Renderless Components in Vue.js" article and "scoped slots" approach described there. This approach is really ugly.

There were many issues about sharing things and logic between the components in Svelte. For example, I supposed the mixins as an alternative to classical inheritance. But seems it's an ongoing discussion with unpredictable results.

Btw, in Ractive we've many ways to share logic and keep our code DRY:

Classical inheritance

import ParentComponent from '...';

export default ParentComponent.extend({ ... });

Extend using an existing constructor or class

import Ractive from 'ractive';
import ParentClass from '...';

export default Ractive.extendWith(ParentClass, ({ ... });

Mixins

import Ractive from 'ractive';
import mixinOne from '...';
import mixinTwo from '...';

export default Ractive.extend({}, mixinOne, mixinTwo);

Custom plugins

import Ractive from 'ractive';
import customPlugin from '...';

// apply plugin to constructor
Ractive.use(customPlugin({...}));

// apply plugin to instance
const r = new Ractive();
r.use(customPlugin({...}));
Kiho commented 6 years ago

I have several components have exactly same but different input component in slot. I don't have much idea on how to but I hope I can share code in those components. Maybe HOC is best fit here.

TextField

<Field :uuid :label bind:submit bind:error >
    <TextInput {{...settings}} bind:value bind:submit bind:error />
</Field>

<script>
    import Field from './Field.html';
    import TextInput from './inputs/TextInput.html';
    import fieldBase from './inputs/field-base';
    export default {
        components: {
            Field,
            TextInput
        },
        data: fieldBase.fieldData,
        oncreate() {
            this.set({ settings: this.get(), uuid: fieldBase.makeUniqueId() });
        }, 
    }
</script>

SelectField

<Field :uuid :label bind:submit bind:error >
    <SelectInput {{...settings}} bind:value bind:submit bind:error />
</Field>

<script>
    import Field from './Field.html';
    import SelectInput from './inputs/SelectInput.html';
    import fieldBase from './inputs/field-base';
    export default {
        components: {
            Field,
            SelectInput
        },
        data: fieldBase.fieldData,
        oncreate() {
            this.set({ settings: this.get(), uuid: fieldBase.makeUniqueId() });
        },
    }
</script>

And I could have a lot more field components in this project https://github.com/Kiho/svelte-formgrid

TehShrike commented 6 years ago

@Kiho that seems like a great place for a <:Component> tag.

Kiho commented 6 years ago

@TehShrike I thought that too but does not work well for me... I tried something here but not too good https://github.com/Kiho/svelte-formgrid/blob/dynamicfield/src/TextField.html Could be much better if this https://github.com/sveltejs/svelte/issues/1303 is resolved.
Anyway my goal is, I like to have TexInput as stand-alone, part of data-grid, part form-grid, and Field+TextInput -> TextField I am OK with other implementations but TextField I hope that svelte has a way to share state between Field and TextInput component. Something like this syntax or maybe just script only.

<:Self />

<script>
    import Field from './Field.html';
    import { TextInput } from './inputs';

    export default Field(TextInput);
</script>
TehShrike commented 6 years ago

Something like this doesn't work?

<!-- GenericField -->
<Field :uuid :label bind:submit bind:error >
    <:Component {input} {{...settings}} bind:value bind:submit bind:error />
</Field>

<script>
    import Field from './Field.html';
    import fieldBase from './inputs/field-base';
    export default {
        components: {
            Field,
            TextInput
        },
        data: fieldBase.fieldData,
        oncreate() {
            this.set({ settings: this.get(), uuid: fieldBase.makeUniqueId() });
        }, 
    }
</script>
<!-- TextField -->
<GenericField input={{TextInput}} bind:value />

<script>
    import TextInput from './inputs/TextInput.html';
    import GenericField from './inputs/GenericField.html';

    export default {
        data() {
            return {
                TextInput,
            }
        }
        components: {
            GenericField,
        }
    }
</script>
<!-- SelectField -->
<GenericField input={{SelectField}} bind:value />

<script>
    import SelectField from './inputs/SelectField.html';
    import GenericField from './inputs/GenericField.html';

    export default {
        data() {
            return {
                SelectField
            }
        },
        components: {
            GenericField,
        }
    }
</script>

I like this method personally, though there's an argument to be made for adding sugar to make the TextField/SelectField-style compositions shorter.

Kiho commented 6 years ago

That almost work but not this time. We need something like this.

<!-- TextField -->
<GenericField {{...this}} input={{TextInput}} bind:value />
TehShrike commented 6 years ago

Right. I'm also excited for https://github.com/sveltejs/svelte/issues/1303.

Kiho commented 6 years ago

@TehShrike Now I made it much simpler with this Field component. https://github.com/Kiho/svelte-formgrid/blob/master/src/Field.html TextField

<Field {settings} {fieldtype} bind:value />

<script>
    import fieldBase from './inputs/field-base';
    import { TextInput } from './inputs';

    export default {
        components: {
            Field: './Field.html'
        },
        data: () => fieldBase.fieldData({ fieldtype: TextInput }),
        oncreate() {
            this.set({ settings: this.get() });
        }, 
    }
</script>

NumberField

<Field {settings} {fieldtype} bind:value />

<script>
    import fieldBase from './inputs/field-base';
    import { NumberInput } from './inputs';

    export default {
        components: {
            Field: './Field.html'
        },
        data: () => fieldBase.fieldData({ fieldtype: NumberInput }),
        oncreate() {
            this.set({ settings: this.get() });
        }, 
    }
</script>

Only the difference in those two are TextInput & NumberInput but I could not find a way to simplify more. Can we do something like this?

<svelte:compose  base="./base-field.html" fieldtype="./inputs/NumberInput.html" />

BaseField

<Field {settings} {fieldtype} bind:value />

<script>
    import fieldBase from './inputs/field-base';

    export default {
        components: {
            Field: './Field.html'
        },
        data: () => fieldBase.fieldData(),
        oncreate() {
            this.set({ settings: this.get() });
        }, 
    }
</script>
Kiho commented 6 years ago

This works and good enough for me. It make code size much smaller.

import Field from './Field.html';
import { TextInput, NumberInput, MaskedInput, CurrencyInput, SelectInput } from './inputs';

function mergeState(data, fieldtype) {
    return Object.assign({}, data, { settings: data }, { fieldtype });
}

export const TextField = class extends Field {
    constructor(options) {
        options.data = mergeState(options.data, TextInput);
        super(options);
    }    
}

export const NumberField = class extends Field {
    constructor(options) {
        options.data = mergeState(options.data, NumberInput);
        super(options);
    }    
}
............

https://github.com/Kiho/svelte-formgrid/blob/v2-class/src/fields.js

Conduitry commented 5 years ago

I believe this can be closed. There is less boilerplate involved in v3 with e.g. reusable lifecycle functions, and component composition is a lot more powerful.

tomcon commented 5 years ago

from what I've observed I'm not 100% certain this is true. Haven't had time to use v3 with a vengeance yet though so don't know for sure and may be wrong, but the v2 inability to inherit or use mixins (not easy at all in v2, impossible in v3?) seems to hold true for v3

stalkerg commented 5 years ago

@Conduitry I believe we should reopen this issue. This issue was about shared logic between the component and not about "reusable lifecycle functions". Even more for Svelte v2 we found few ways to shared logic BUT in Svelte 3 you break these ways because we can't get the link to the current component.

stalkerg commented 5 years ago

@tomcon looks like Svelte v3 only add new obstacles to the main problem, currently, you have no "this".

Kiho commented 5 years ago

I am in same boat, I wrote big application last year but I could not find a way to move to v3.

stalkerg commented 5 years ago

@Kiho I also have huge Svelte v2 application, around 100 fat components and many other files with logic. The main problem is my application in production but supports Svelte v2 have already dropped and I have no choice, I should migrate to the new version.