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

feat: add hook to transform h's arguments #851

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

JessicaSachs
Copy link
Contributor

@JessicaSachs JessicaSachs commented Mar 17, 2020

Context

In Vue Test Utils (VTU) we provide two core features: “Shallow Mount” and “Stubs”.

Both shallowMount and stubs enable the user to short circuit rendering a component tree. You can opt into this feature on any mount command (using stubs) or shallowMount command (stubs everything).

This PR adds the necessary functions to enable Vue Test Utils to implement shallowMount and stubs for Vue 3.

Implementation Details

Existing shallowMount functionality in VTU w/ Vue 2
shallowMount is implemented by overwriting $createElement in a beforeCreate hook and conditionally choosing to apply the real $createElement. In VTU we are able to support shallowMounting parent components that use the h function directly.

Implementing shallowMount in Vue 3
Identical implementation using beforeCreate hook is not possible because h is not modifiable for a parent component instance like it was in Vue 2.

Proposed API

Conditionally overload h’s functionality when its parent context wants to stub the component. Do this by using a transformer function to mutate the arguments passed into h.

  1. This function is called transformHArgs and it returns an array of arguments to be applied to all h functions.
  2. Add the ability to reset transformHArgs with a reset function.

Example Usage
Statically override all calls to h to return an h1 tag (this isn't useful, but it demonstrates the API)

import { transformHArgs, h } from '@vue/runtime-core/src/h'
transformHArgs(() => ['h1', 'Hello World'])
h('div', 'Hello') // renders a vnode equivalent to <h1>Hello World</h1>

Conditionally modify what is rendered

import { transformHArgs, h } from '@vue/runtime-core/src/h'
transformHArgs((hArgs, instance) => {
    // if the component is in the "stubs" list for Vue Test Utils
    //     return [`stub-${instance.type.name}`]
    // return hArgs // Passthrough to the real h function
})

h({ name: 'my-component' , template: '<span/>' }) // renders <stub-my-component/> if it is found in the "stubbed list"

Questions

  • Should this API be exported from @vue/runtime-core?
  • Thoughts on how to avoid querying document.body in the test for the ComponentInternalInstance? Do we have an element reference available on the vm still?

@lmiller1990
Copy link
Member

lmiller1990 commented Mar 18, 2020

Great progress! I think we will need to update src/index.ts to export this:

export { 
  h, 
  + resetTransformHArgs,  
  + transformHArgs 
} from './h'

Then we can just do import { transformHArgs } from 'vue'. Unless we don't want to expose this so easily?


No luck using this so far. Minimal repro of what I tried:

  1. cd into packages/runtime-core
  2. updated index.ts to export the transform functions like suggested above
  3. yarn build runtime-core
  4. set up a simple babel-node env to be able to confuse esm-bundler.js (also tried with raw node, same result as below)
  5. do this
import { transformHArgs, h } from './dist/runtime-core.esm-bundler.js'

transformHArgs(() => ['div'])

console.log(
  h('span')
)

Final console.log returns a bunch a vnode like so:

{
  _isVNode: true,
  type: 'div',
  props: null,
  // ...
}

Once this is working, it should be type: span.

No luck overriding the arguments so far... Hmm. Seems like once you do import { h }, even if you transform args, the original h is still "cached" or something, and the override does nothing. I didn't even know you could redefine ES modules dynamically like this (maybe you can't? I thought they were static and immutable).

I'll keep playing around.

@@ -159,3 +160,23 @@ export function h(type: any, propsOrChildren?: any, children?: any): VNode {
return createVNode(type, propsOrChildren, children)
}
}

const originalH: Function = exports.h
Copy link
Member

Choose a reason for hiding this comment

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

exports is only available in Node.js environments (the tests pass because jest runs the tests in Node.js) and won't work when bundled.

What we should here is let transformHArgs save the transformer in a local variable, and check for the existence of that variable inside h. We should probably also wrap the check in a if (__DEV__) block.

Copy link
Member

@lmiller1990 lmiller1990 Mar 18, 2020

Choose a reason for hiding this comment

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

Right, that makes more sense. So something like:

let tranformedArgs

export function tranformHArgs(...args) {
  tranformedArgs = [...args]
}

export function resetHArgs(...args) {
  tranformedArgs = undefined
}

// other stuff

export function h(type: any, propsOrChildren?: any, children?: any): VNode {
  if (__DEV___ || __TEST__ && transformedArgs) {
    return createVNode(...transformedArgs)
  }

  // current implementation here
    if (arguments.length === 2) {
      if (isObject(propsOrChildren) && !isArray(propsOrChildren)) {
      // ...
}

Which is more or less the same concept Jess was going for. Is what you are suggesting @yyx990803 ?

This will have a limitation we cannot run many tests in parallel, since they all share the same state. I think we might just have to live with that, I don't see another easy option that doesn't involve a lot of changes.

Thanks for the feedback.

Copy link
Member

@yyx990803 yyx990803 Mar 18, 2020

Choose a reason for hiding this comment

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

Well I was thinking of something more like

let argsTransformer

export function transformHArgs(transformer) {
  argsTransformer = transformer
}

function originalH(...) {}

export const h = __DEV__
  ? (...args) => {
    if (argsTransformer) {
      args = argsTransformer(args, currentRenderingInstance)
    }
    return originalH(...args)
  }
  : originalH

Which should not be subject to concurrency issues. Also you will call this only once when VTU is loaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha! V-nice, will update ⚡️

@JessicaSachs JessicaSachs force-pushed the feat/support-transformer-for-h branch from 4c5d25c to 8496c09 Compare March 19, 2020 03:27
@JessicaSachs JessicaSachs force-pushed the feat/support-transformer-for-h branch from 8496c09 to f1f676a Compare March 19, 2020 04:51
@yyx990803 yyx990803 merged commit b7d1e0f into vuejs:master Mar 19, 2020
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