feat: add scoped slots option #507

38elements · 2018-04-01T12:49:34Z

This is related to #156 .
This adds scopedSlots option by replacing the renderSlot method.
It is supported at [email protected]+.
It does not support template tag.

eddyerburgh

Looks good! I don't like monkey patching the _t helper but we can look at changing that in the future. For now we can go ahead with this. I've added some comments

eddyerburgh · 2018-04-04T18:10:02Z

test/specs/mounting-options/scopedSlots.js

+    it('mounts component scoped slots', () => {
+      const wrapper = mountingMethod(ComponentWithScopedSlots, {
+        scopedSlots: {
+          'item': '<p slot="item" slot-scope="props">{{props.index}},{{props.text}}</p>'


Do we need to include slot=item in the markup? We already identify it as the item slot in the scopedSlots object

eddyerburgh · 2018-04-04T18:11:09Z

packages/test-utils/types/index.d.ts

@@ -113,6 +113,7 @@ interface MountOptions<V extends Vue> extends ComponentOptions<V> {
  localVue?: typeof Vue
  mocks?: object
  slots?: Slots
+  scopedSlots?: string
  stubs?: Stubs,


This should be an object. Can you add a test for this in types/test?

eddyerburgh · 2018-04-04T18:13:07Z

test/specs/mounting-options/scopedSlots.js

+
+describeWithShallowAndMount('scopedSlots', (mountingMethod) => {
+  if (vueVersion >= 2.5) {
+    it('mounts component scoped slots', () => {


We have an itIf helper function in resouces/utils. Could you use that instead of if else statement

38elements · 2018-04-05T03:02:50Z

Thank you for reviewing.
I changed this.

ktsn · 2018-04-05T03:12:43Z

packages/test-utils/types/index.d.ts

@@ -113,6 +113,7 @@ interface MountOptions<V extends Vue> extends ComponentOptions<V> {
  localVue?: typeof Vue
  mocks?: object
  slots?: Slots
+  scopedSlots?: object


We can specify more detailed type Record<string, string>

Thank you for pointing out.
I changed it.

eddyerburgh · 2018-04-06T17:11:57Z

docs/en/api/options.md

+
+Provide an object of scoped slots contents to the component. The key corresponds to the slot name. The value can be a template string.
+
+There is two limitations.  


There is two limitations > There are two limitations.

eddyerburgh · 2018-04-06T17:13:06Z

docs/en/api/options.md

+
+There is two limitations.  
+
+* This supports [email protected]+.


This option is only supported in [email protected]+

eddyerburgh · 2018-04-06T17:16:49Z

docs/en/api/options.md

+
+* This supports [email protected]+.
+
+* You can not set a `template` tag to top of `scopedSlots` option.


You cannot use <template> tag as the root element in thescopedSlots option.

eddyerburgh · 2018-04-06T17:17:40Z

packages/create-instance/add-scoped-slots.js

+  Object.keys(scopedSlots).forEach((key) => {
+    const template = scopedSlots[key].trim()
+    if (template.substr(0, 9) === '<template') {
+      throwError('scopedSlots option does not support template tag.')


Can the error message read:

the scopedSlots option does not support a tag as the root element.

eddyerburgh · 2018-04-06T17:18:49Z

packages/create-instance/create-instance.js

+      vm._renderProxy._t = function (name, feedback, props, bindObject) {
+        const scopedSlotFn = vm.$_vueTestUtils_scopedSlots[name]
+        if (scopedSlotFn) {
+          props = { ...bindObject, ...props }


Why do we need to set vm._renderProxy.props?

scopedSlotFn needs vm._renderProxy.props.
If vm._renderProxy.prop is not setted, the render function can not use props.

with(this){return _c('p',{},[_v(_s(props.index)+","+_s(props.text))])}

There is a bug.
When slot-scope is other than props, this is not work.

38elements · 2018-04-07T02:36:24Z

Thank you for pointing out.
I changed them.

38elements · 2018-04-08T03:42:32Z

There is a bug.
When slot-scope is other than props, this is not work.
I will fix it.

38elements · 2018-04-08T10:37:43Z

packages/create-instance/create-instance.js

+              proxy[key] = vm._renderProxy[key]
+            }
+          })
+          proxy[slotScope] = props


Since when slot name and slot scope is same, vm._renderProxy.[<slot name>] is changed in _t function, proxy is created.

<ul> <slot name="foo" v-for="(item, index) in foo" :text="item.text" :index="index"> </slot> </ul>

function render() { with(this) { return _c('ul', [_l((foo), function (item, index) { return _t("foo", null, { text: item.text, index: index }) })], 2) } }

scopedSlotFn

function anonymous() { with(this){return _c('p',{},[_v(_s(foo.index)+","+_s(foo.text))])} }

Mattchewone · 2018-04-13T11:17:39Z

Looking forward to this being merged.

Thanks for this.

38elements · 2018-04-14T02:16:19Z

packages/create-instance/create-instance.js

-            if (key[0] === '_') {
-              proxy[key] = vm._renderProxy[key]
-            }
+          const helpers = ['_c', '_o', '_n', '_s', '_l', '_t', '_q', '_i', '_m', '_f', '_k', '_b', '_v', '_e', '_u', '_g']


Object.keys(vm._renderProxy).concat(Object.keys(Vue.prototype)).forEach((key) => { may be better,
since it is abstruct.

eddyerburgh

Looking good, this adds some much needed support 👍 . I've added a couple of comments

eddyerburgh · 2018-04-14T07:45:25Z

packages/create-instance/add-scoped-slots.js

+      throwError('the scopedSlots option does not support a template tag as the root element.')
+    }
+    const domParser = new window.DOMParser()
+    const document = domParser.parseFromString(template, 'text/html')


Can you rename this variable to avoid collisions with window.document?

I changed it.

eddyerburgh · 2018-04-14T07:50:50Z

packages/create-instance/create-instance.js

+          helpers.forEach((key) => {
+            proxy[key] = vm._renderProxy[key]
+          })
+          proxy[slotScope] = props


Does Vue throw an error if there are two conflicting slot scopes? If not, we should throw an error here

It is impossible to set a word that starts with "_" in Vue.

[Vue warn]: Property or method "_foo" is not defined on the instance but referenced during render. Make sure that this property is reactive, either in the data option, or for class-based components, by initializing the property. See: https://vuejs.org/v2/guide/reactivity.html#Declaring-Reactive-Properties.

I meant if you mounted with this option:

{ scopedSlots: { foo: '<div />', foo: '<input />' } }

The second scopedSlot would overwrite the first scopedSlots in _vueTestUtils_slotScopes. Although thinking about it, this is probably fine.

It is not possible to register the same key.

{ foo: 1, foo: 2 } // => { foo: 2 }

🤦‍♂️ Of course ;p

eddyerburgh · 2018-04-14T14:13:45Z

docs/en/api/options.md

+* You can not use `<template>` tag as the root element in the `scopedSlots` option.
+
+* This does not support PhantomJS.  
+Please use [Puppeteer](https://github.com/karma-runner/karma-chrome-launcher#headless-chromium-with-puppeteer).


Can you change to:

You can use Puppeteer as an alternative.

38elements · 2018-04-14T14:32:00Z

Thank you for pointing out.
I changed it.

eddyerburgh · 2018-04-14T14:34:54Z

packages/create-instance/create-instance.js

@@ -61,7 +61,7 @@ export default function createInstance (

  if (options.scopedSlots) {
    if (window.navigator.userAgent.match(/PhantomJS/i)) {
-      throwError('the scopedSlots option does not support strings in PhantomJS. Please use Puppeteer, or pass a component.')
+      throwError('the scopedSlots option does not support in PhantomJS. Please use Puppeteer, or pass a component.')


the scopedSlots option does not support in PhantomJS. Please use Puppeteer, or pass a component.
to

the scopedSlots option does not support PhantomJS. Please use Puppeteer, or pass a component.

I changed it.

eddyerburgh

Great job 👍

38elements · 2018-04-14T15:31:53Z

@eddyerburgh
Thank you for reviewing.

Haroenv · 2018-10-22T11:07:14Z

@38elements, what was the reason <template> in scoped slot was not implemented?

38elements changed the title ~~feat: add scoped slots option~~ [WIP]feat: add scoped slots option Apr 1, 2018

38elements mentioned this pull request Apr 1, 2018

Test slot scope #156

Closed

feat: add scoped slots option

cecf8eb

38elements changed the title ~~[WIP]feat: add scoped slots option~~ feat: add scoped slots option Apr 2, 2018

38elements added 2 commits April 3, 2018 19:12

add scopedSlots to index.d.ts

e7da1f7

add test

37c5950

eddyerburgh requested changes Apr 4, 2018

View reviewed changes

38elements added 7 commits April 5, 2018 08:22

use Object.assign()

3c841a1

fix types

ea2d16f

change test

8f3239f

fix document

d4e7bdf

refactor

e6ad08d

add test for slots

8da1a4e

refactor

7665971

ktsn reviewed Apr 5, 2018

View reviewed changes

38elements and others added 3 commits April 5, 2018 12:20

improve types

0953171

use spread syntax

f737344

Update options.md

d925dcf

eddyerburgh reviewed Apr 6, 2018

View reviewed changes

38elements and others added 2 commits April 7, 2018 11:00

Update options.md

58399c5

change message

4678bfb

38elements added 2 commits April 8, 2018 18:50

fix props

032513c

fix test condition

a1f008c

38elements commented Apr 8, 2018

View reviewed changes

38elements added 2 commits April 9, 2018 11:12

add helpers

cb000ea

update packages/server-test-utils/types/index.d.ts

4690a6a

38elements commented Apr 14, 2018

View reviewed changes

eddyerburgh requested changes Apr 14, 2018

View reviewed changes

document -> _document

73a7e5f

eddyerburgh reviewed Apr 14, 2018

View reviewed changes

update message

125c39c

eddyerburgh reviewed Apr 14, 2018

View reviewed changes

fix message

303d1a6

eddyerburgh approved these changes Apr 14, 2018

View reviewed changes

eddyerburgh merged commit e6a54b2 into vuejs:dev Apr 14, 2018

38elements deleted the scopedslots branch April 15, 2018 04:26

Haroenv mentioned this pull request Oct 22, 2018

feat(MenuSelect): add item slot algolia/vue-instantsearch#555

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add scoped slots option #507

feat: add scoped slots option #507

38elements commented Apr 1, 2018 •

edited

Loading

eddyerburgh left a comment

eddyerburgh Apr 4, 2018

eddyerburgh Apr 4, 2018

eddyerburgh Apr 4, 2018

38elements commented Apr 5, 2018

ktsn Apr 5, 2018

38elements Apr 5, 2018

eddyerburgh Apr 6, 2018

eddyerburgh Apr 6, 2018

eddyerburgh Apr 6, 2018

eddyerburgh Apr 6, 2018

eddyerburgh Apr 6, 2018 •

edited

Loading

38elements Apr 7, 2018 •

edited

Loading

38elements Apr 8, 2018

38elements commented Apr 7, 2018 •

edited

Loading

38elements commented Apr 8, 2018

38elements Apr 8, 2018 •

edited

Loading

Mattchewone commented Apr 13, 2018

38elements Apr 14, 2018

eddyerburgh left a comment

eddyerburgh Apr 14, 2018

38elements Apr 14, 2018 •

edited

Loading

eddyerburgh Apr 14, 2018

38elements Apr 14, 2018 •

edited

Loading

eddyerburgh Apr 14, 2018

38elements Apr 14, 2018

eddyerburgh Apr 14, 2018

eddyerburgh Apr 14, 2018

38elements commented Apr 14, 2018

eddyerburgh Apr 14, 2018

38elements Apr 14, 2018

eddyerburgh left a comment

38elements commented Apr 14, 2018

Haroenv commented Oct 22, 2018


		Provide an object of scoped slots contents to the component. The key corresponds to the slot name. The value can be a template string.

		There is two limitations.


		There is two limitations.

		* This supports [email protected]+.


		* This supports [email protected]+.

		* You can not set a `template` tag to top of `scopedSlots` option.

feat: add scoped slots option #507

feat: add scoped slots option #507

Conversation

38elements commented Apr 1, 2018 • edited Loading

eddyerburgh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

38elements commented Apr 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyerburgh Apr 6, 2018 • edited Loading

Choose a reason for hiding this comment

38elements Apr 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

38elements commented Apr 7, 2018 • edited Loading

38elements commented Apr 8, 2018

38elements Apr 8, 2018 • edited Loading

Choose a reason for hiding this comment

Mattchewone commented Apr 13, 2018

Choose a reason for hiding this comment

eddyerburgh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

38elements Apr 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

38elements Apr 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

38elements commented Apr 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyerburgh left a comment

Choose a reason for hiding this comment

38elements commented Apr 14, 2018

Haroenv commented Oct 22, 2018

38elements commented Apr 1, 2018 •

edited

Loading

eddyerburgh Apr 6, 2018 •

edited

Loading

38elements Apr 7, 2018 •

edited

Loading

38elements commented Apr 7, 2018 •

edited

Loading

38elements Apr 8, 2018 •

edited

Loading

38elements Apr 14, 2018 •

edited

Loading

38elements Apr 14, 2018 •

edited

Loading