Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shared logic between components #779

Closed
stalkerg opened this issue Aug 18, 2017 · 27 comments
Closed

Shared logic between components #779

stalkerg opened this issue Aug 18, 2017 · 27 comments

Comments

@stalkerg
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor

m59peacemaker commented Aug 28, 2017

Related:
#550
#777

@Kiho
Copy link
Contributor

Kiho commented Oct 17, 2017

@Rich-Harris
Copy link
Member

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
Copy link
Contributor Author

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
Copy link

futuraprime commented Feb 20, 2018

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
Copy link
Member

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
Copy link
Member

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

@Rich-Harris
Copy link
Member

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
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

@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
Copy link
Contributor

Kiho commented Apr 4, 2018

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
Copy link
Member

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

@Kiho
Copy link
Contributor

Kiho commented Apr 5, 2018

@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 #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
Copy link
Member

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
Copy link
Contributor

Kiho commented Apr 5, 2018

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

<!-- TextField -->
<GenericField {{...this}} input={{TextInput}} bind:value />

@TehShrike
Copy link
Member

Right. I'm also excited for #1303.

@Kiho Kiho mentioned this issue Apr 12, 2018
@Kiho
Copy link
Contributor

Kiho commented May 5, 2018

@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
Copy link
Contributor

Kiho commented May 6, 2018

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
Copy link
Member

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
Copy link

tomcon commented Apr 16, 2019

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

@Kiho
Copy link
Contributor

Kiho commented Jun 26, 2019

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

@stalkerg
Copy link
Contributor Author

stalkerg commented Jun 26, 2019

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants