feat: stubs mounting option #56

lmiller1990 · 2020-04-13T05:23:48Z

resolves #3!!!! :partyparrot: I'm really happy with this implementation; it feels super clean, main because of the transforVNodeArgs function @JessicaSachs and Evan made in core.

Points of interest:

stubs is now in the global mounting option, since when you stub a component, it stubs them all the way down the tree (same as beta)
the stub is not a DOM node as discussed, but a bare-bones Vue component. Gives us some flexibility for implementing shallowMount.

Breaking change:

We do not support an array of stubs (at least right now). So instead of

stubs: [Foo, Bar]

You need to do:

stubs: {
  Foo: true, Bar: True
}

If this is a bit problem for people we can implement array notation for beta. For alpha, I think this is a good place to leave this and move on to other features.

lmiller1990 · 2020-04-13T05:27:01Z

src/mount.ts

@@ -29,6 +31,7 @@ interface MountingOptions {
    plugins?: Plugin[]
    mixins?: ComponentOptions[]
    mocks?: Record<string, any>


Typing Vue components is very difficult, will need to investigate the Vue codebase to figure out how best to do this

I noticed this too. Am interested to hear about what you find.

lmiller1990 · 2020-04-13T05:29:08Z

src/mount.ts

+        // default stub
+        if (options?.global?.stubs[name] === true) {
+          return [createStub({ name: args[0]['name'] })]
+        }


@dobromir-hristov we one of us gets to working on findComponent we will probably need to pass props as the second arg here, so we can do findComponent(Foo).props). So it will be like

return [createStub({ name: args[0]['name'] }), args[1]] or something. I didn't do it here because I am not exactly sure how this will work yet, but I don't expect it to be a problem.

Yep, we will check it out.

src/mount.ts

dobromir-hristov

Very nice :) Turning up very well.

dobromir-hristov · 2020-04-13T18:58:45Z

src/mount.ts

+
+      if (
+        typeof args[0] === 'object' &&
+        args[0]['name'] in options?.global?.stubs


Probably we should do that magic with the Name transformation here? PascalCase, snake-case, that sort of stuff.

Yeah this is probably a good idea, make it easier to find the stubs, I will implement this

I matched Vue 3, exact name, kebab case, pacal case (aka capital case)

/~https://github.com/vuejs/vue-next/blob/e8a866ec9945ec0464035be4c4c58d6212080a50/packages/runtime-core/src/helpers/resolveAssets.ts#L57

I also did that for the findComponent by name Branch. Need to sync these up later, so we don't duplicate :)

Sure, sounds good!

dobromir-hristov · 2020-04-13T19:01:19Z

src/mount.ts

@@ -133,6 +137,46 @@ export function mount(originalComponent: any, options?: MountingOptions) {
  const { emitMixin, events } = createEmitMixin()
  vm.mixin(emitMixin)

+  // stubs
+  if (options?.global?.stubs) {
+    transformVNodeArgs((args) => {


Maybe we can destructure like this, so its easier to understand context later down?

Suggested change

transformVNodeArgs((args) => {

transformVNodeArgs(([componentOptions]) => {

Thanks for the suggestion, but I am not really a fan of this refactor. This variable can be a string (eg 'div'), or an object of options, so componentOptions is a bit misleading. I guess the ideal name is htmlTagOrComponentOptions. I will just put a comment here for now.

dobromir-hristov · 2020-04-13T19:03:35Z

src/mount.ts

+
+        // custom stub implementation
+        if (typeof options?.global?.stubs[name] === 'object') {
+          return [options?.global?.stubs[name]]


Suggested change

return [options?.global?.stubs[name]]

return [options.global.stubs[name]]

dobromir-hristov · 2020-04-13T19:07:40Z

src/mount.ts

+      ) {
+        const name = args[0]['name']
+        // default stub
+        if (options?.global?.stubs[name] === true) {


You are already in the conditional check, its there.

Suggested change

if (options?.global?.stubs[name] === true) {

if (options.global.stubs[name] === true) {

heh, copy pasted... thanks, I'll clean this up

JessicaSachs

Did we discuss dropping support for render functions? If we’re dropping support for render functions, how does this affect JSX unit tests?

In general, I like this implementation. I have some of the same cleanup nits that Dob and Adria do.

I am concerned about the lack of support for JSX/render functions.

lmiller1990 · 2020-04-14T11:16:33Z

@JessicaSachs we support render functions. You can do this:

stubs: {
  Foo: { render() { return h('div') } 
}

etc. There is some tests for this.

We will support JSX/TSX too. I am not sure on the status of JSX/TSX support yet.

lmiller1990 · 2020-04-14T11:25:17Z

src/utils.ts

+import upperFirst from 'lodash/upperFirst'
+import flow from 'lodash/flow'
+
+export const pascalCase = flow(camelCase, upperFirst)


Lodash told me to do it this way lodash/lodash#3102

afontcu · 2020-04-14T12:00:41Z

.github/workflows/ci.yml

+    - run: yarn install
+    - run: yarn lint
+    - run: yarn test
+    - run: yarn build


lmiller1990 · 2020-04-14T13:35:17Z

Going to merge this up. I fully expect to revisit this code soon, probably for shallow.

lmiller1990 added 5 commits April 13, 2020 13:24

wip: stubs

0db36c3

wip: tests for stubs

361019d

feat: stubs

d4897f1

chore: add comment to clarify behavior

61b5e2b

fix: reset h args each test

11f47af

lmiller1990 requested review from JessicaSachs, afontcu and dobromir-hristov April 13, 2020 05:23

lmiller1990 commented Apr 13, 2020

View reviewed changes

lmiller1990 added 2 commits April 13, 2020 15:37

feat: support sfc as a stub

d082064

tests: add tests for inline stubs

d515f3f

lmiller1990 mentioned this pull request Apr 13, 2020

wip: shallow #21

Closed

afontcu reviewed Apr 13, 2020

View reviewed changes

src/mount.ts Outdated Show resolved Hide resolved

dobromir-hristov approved these changes Apr 13, 2020

View reviewed changes

JessicaSachs reviewed Apr 14, 2020

View reviewed changes

lmiller1990 added 5 commits April 14, 2020 18:22

refactor: extract functions to improve readability

1a5024e

fix: support kebab case

d14dc59

tests: add test for pascal case component name

2355f55

tests: stub component with camelcase naming

1c65bf7

refactor: add comments

485b1ac

lmiller1990 commented Apr 14, 2020

View reviewed changes

lmiller1990 added 3 commits April 14, 2020 21:31

refactor: update rollup build

126d5dd

chore: rollup config

de4e38d

chore: workflow

da5d3cb

afontcu reviewed Apr 14, 2020

View reviewed changes

lmiller1990 merged commit 08e2881 into master Apr 14, 2020

lmiller1990 deleted the feature/stubs branch April 14, 2020 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: stubs mounting option #56

feat: stubs mounting option #56

lmiller1990 commented Apr 13, 2020 •

edited

Loading

lmiller1990 Apr 13, 2020

JessicaSachs Apr 14, 2020

lmiller1990 Apr 13, 2020 •

edited

Loading

dobromir-hristov Apr 13, 2020

dobromir-hristov left a comment

dobromir-hristov Apr 13, 2020

lmiller1990 Apr 14, 2020

lmiller1990 Apr 14, 2020

dobromir-hristov Apr 14, 2020

lmiller1990 Apr 14, 2020

dobromir-hristov Apr 13, 2020

lmiller1990 Apr 14, 2020

dobromir-hristov Apr 13, 2020

lmiller1990 Apr 14, 2020

dobromir-hristov Apr 13, 2020

lmiller1990 Apr 14, 2020

JessicaSachs left a comment

lmiller1990 commented Apr 14, 2020 •

edited

Loading

lmiller1990 Apr 14, 2020

afontcu Apr 14, 2020

lmiller1990 commented Apr 14, 2020

	transformVNodeArgs((args) => {
	transformVNodeArgs(([componentOptions]) => {

	return [options?.global?.stubs[name]]
	return [options.global.stubs[name]]

	if (options?.global?.stubs[name] === true) {
	if (options.global.stubs[name] === true) {

feat: stubs mounting option #56

feat: stubs mounting option #56

Conversation

lmiller1990 commented Apr 13, 2020 • edited Loading

Points of interest:

Breaking change:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 Apr 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dobromir-hristov 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JessicaSachs left a comment

Choose a reason for hiding this comment

lmiller1990 commented Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 commented Apr 14, 2020

lmiller1990 commented Apr 13, 2020 •

edited

Loading

lmiller1990 Apr 13, 2020 •

edited

Loading

lmiller1990 commented Apr 14, 2020 •

edited

Loading