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.

Sign up for GitHub

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

feat: add scoped slots option #507

Merged
merged 22 commits into from
Apr 14, 2018
Merged

feat: add scoped slots option #507

Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cecf8eb
feat: add scoped slots option
38elements Apr 1, 2018
e7da1f7
add scopedSlots to index.d.ts
38elements Apr 3, 2018
37c5950
add test
38elements Apr 4, 2018
3c841a1
use Object.assign()
38elements Apr 4, 2018
ea2d16f
fix types
38elements Apr 4, 2018
8f3239f
change test
38elements Apr 5, 2018
d4e7bdf
fix document
38elements Apr 5, 2018
e6ad08d
refactor
38elements Apr 5, 2018
8da1a4e
add test for slots
38elements Apr 5, 2018
7665971
refactor
38elements Apr 5, 2018
0953171
improve types
38elements Apr 5, 2018
f737344
use spread syntax
38elements Apr 5, 2018
d925dcf
Update options.md
38elements Apr 5, 2018
58399c5
Update options.md
38elements Apr 7, 2018
4678bfb
change message
38elements Apr 7, 2018
032513c
fix props
38elements Apr 8, 2018
a1f008c
fix test condition
38elements Apr 8, 2018
cb000ea
add helpers
38elements Apr 9, 2018
4690a6a
update packages/server-test-utils/types/index.d.ts
38elements Apr 11, 2018
73a7e5f
document -> _document
38elements Apr 14, 2018
125c39c
update message
38elements Apr 14, 2018
303d1a6
fix message
38elements Apr 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions packages/create-instance/create-instance.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,9 @@ export default function createInstance (
if (scopedSlotFn) {
props = { ...bindObject, ...props }
Copy link
Member

@eddyerburgh eddyerburgh Apr 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@38elements 38elements Apr 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))])}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

const proxy = {}
Object.keys(vm._renderProxy).concat(Object.keys(Vue.prototype)).forEach((key) => {
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']
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

helpers.forEach((key) => {
proxy[key] = vm._renderProxy[key]
})
proxy[slotScope] = props
Copy link
Contributor Author

@38elements 38elements Apr 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))])}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@38elements 38elements Apr 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not possible to register the same key.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️ Of course ;p

return scopedSlotFn.call(proxy)
Expand Down