Skip to content

[WIP] Spread attributes and properties #1248

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

Merged
merged 3 commits into from
Apr 1, 2018
Merged

Conversation

mrkishi
Copy link
Member

@mrkishi mrkishi commented Mar 16, 2018

So, I'm afraid I'm not able to get this feature to production because I'm severely lacking in the internals knowledge, but here's a PR you can play with.

It adds spread attributes/properties to both Component and Element tags, with restrictions previously explored here: only one spread per tag and named attributes have precedence over them.

There's no SSR at all and lots of code duplication. I'm still confused about the particularities of contextualise, the split between Components and Elements in the source and how to reuse code between them. I based this off attributes and, as far as I can tell, they too are not very reusable with the implementation split between Attribute and Component.

If anyone has tips on where to take this, I'll surely give it a try, but otherwise feel free to discard it with a proper implementation!

@Rich-Harris
Copy link
Member

Thanks for tackling this @mrkishi, this will be an awesome feature if we pull it off. Will study the PR in more detail later when I get a moment, but it looks good on an initial read.

I'm still confused about the particularities of contextualise

Me too. That whole part of the codebase is kind of a rat's nest, I'm itching to refactor it.

@Rich-Harris Rich-Harris merged commit 0177418 into sveltejs:master Apr 1, 2018
@bestguy
Copy link

bestguy commented Apr 14, 2018

Hi, thanks for this. Is this in latest release (1.60.x) ?
If so, it's not clear how to use it. Was hoping to use this for on:* event handlers.
I'd like to be able to define reusable components like this:

<Button
   class="btn btn{{outline ? '-outline' : ''}}-{{color}}{{size ? ` btn-${size}` : ''}}{{block ? ' btn-block' : ''}}           {{ active ? ' active' : ''}}{{ disabled ? ' disabled' : ''}}"
   {{...props}}
 >
   {{yield}}
 </Button>

Then users can add whatever props they need, like aria, data, on: event handlers, etc:

<Button on:click="..." on:hover="...">Click Me</Button>

@Rich-Harris
Copy link
Member

@bestguy yes, it's in the latest release. It only works for attributes, not directives like on:click. The easiest way to forward events is to use this shorthand...

<button on:click>click me</button>

...which is equivalent to

<button on:click="fire('click', event)">click me</button>

Note that {{yield}} was deprecated in favour of <slot></slot>, to align with the platform.

@bestguy
Copy link

bestguy commented Apr 14, 2018

Okay cool, thanks Rich. Am in process of updating to latest Svelte goodies, this helps

@mrkishi mrkishi deleted the spread branch April 15, 2018 02:49
7nik pushed a commit to 7nik/svelte that referenced this pull request Apr 9, 2025
* WIP

* fix

* tidy up

* tidy up

* put npm stuff in separate file

* tidy up

* remove ./

* tidy up

* fix

* lint

* fix

* always use same svelte version

* fix pkg.pr.new versions

* use jsDelivr for more stuff

* remove packages_url stuff

* lint

* gah

* fix `local`

* lint

* gah shut up typescript

* remove unused stuff

* tidy up

* simplify some stuff

* unused

* only create tutorial bundler once

* lint

* tidy up

* unused

* move packages/editor into packages/repl, where it belongs

* lint

* DRY out

* less reliance on globals

* simplify

* tighten up

* tidy up

* fix

* bundler already discards stale results

* drive-by fix

* migrate component

* get rid of `$bundle` store

* tidy up

* better status message

* lint

* reinstate imports array

* lint

* use jsDelivr API instead of fetching package.json

* better error messages

* use onversion mechanism

* remove

* use raw state

* fix

* explanatory comment
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

Successfully merging this pull request may close these issues.

3 participants