vuejs / test-utils

Vue Test Utils for Vue 3
https://test-utils.vuejs.org
MIT License
1.04k stars 244 forks source link

setData prohibits changing attrs #778

Closed yankeeinlondon closed 2 years ago

yankeeinlondon commented 3 years ago

I have a composable utility whose responsibility is to intercept the incoming class property from the context object and then parse and distribute it into various "regions". In any event, the goal is to reactively respond to changes when they occur and while this would be a relatively simple matter on props which are in fact reactive, the context object is just a POJO so to achieve this I've tapped into the onUpdated() event hook and I'm fairly certain my implementation will be reactive but of course I need a test for this. Sadly, it would appear, that the setData() call is either not working or at least not working as I'd expect it to having read the docs.

So to set the scene, I have a utlity function which gives me a pointer to a TestComponent and let's me set the initial state of the class to whatever I like. The TestComponent then uses my class parser functionality and this has been tested pretty thoroughly without any need for using the test library:

function testComponent(klass = "", options: Parameters<typeof shallowMount>[1] = {}) {
  klass = options.attrs?.class ? [(options.attrs as { class: string }).class, klass].join(" ") : klass;
  options = { ...options, attrs: { ...options.attrs, class: klass } };
  return shallowMount(TestComponent, options).findComponent(TestComponent);
}

As a "for instance", this test passes without issue:

it("tokenization regions are all populated correctly", () => {
  const c = testComponent(DEFAULT_CLASS);
  const tokens = c.vm.tokens;

  expect(tokens.root).toBe("my-component bg-blue-100");
  expect(tokens.content).toBe("text-gray-300");
  expect(tokens.title).toBe("opacity-0.9");
});

DEFAULT_CLASS is "my-component text-gray-300 title:opacity-0.9 bg-blue-100"

However, when I try to test for my parsers ability to detect change and provide reactive parsing it fails at any call to setData(). Here's the test where this happens:

it("reactivity of 'class' observed on mounted test component", async () => {
  const c = testComponent(DEFAULT_CLASS);
  const initial = c.vm.tokens;
  // initial state
  expect(initial.title).toBe("opacity-0.9");

  // mutate the "class" property
  await c.setData({ attrs: { class: DEFAULT_CLASS.replace("opacity-0.9", "opacity-1") } });

  // validate that tokens have been updated
  const changed = c.vm.tokens;
  expect(changed.title).toBe("opacity-1");
});

the error returned is "TypeError: Cannot add property attrs, object is not extensible"

Now I accept that there is a possibility that I've misinterpreted the use of this API call but with the info I have it would appear this should work and this type of test is very important. Especially now that Tailwind CSS and class-based styling has become so popular. I have always been a bit surprised by even the core VueJS docs are a bit light on addressing this important edge case so people don't make the assumption that ctx is reactive but surely there must be a good way of testing this?

yankeeinlondon commented 3 years ago

FYI, here's the implementation of TestComponent for context purposes even though I suspect it's implementation plays no role in the error being discussed:

const TestComponent = defineComponent({
  name: "TestComponent",
  inheritAttrs: false,
  setup(props, ctx) {
    const { tokenize, delegate } = useTailwind(props, ctx);
    const tokens = tokenize("title", delegate("content", "textColor"));
    return { tokens, ctx };
  },
  template: `<p>root: {{tokens.root}}</p><p>title: {{tokens.title}}</p><p>root: {{tokens.content}}</p>`
});

and then useTailwind looks like this:

export function useTailwind<T extends Record<string, E>, E extends EmitsOptions>(props: T, ctx: SetupContext<E>): IUseTailwind<T, E> {
  const klass = ref<string>(typeof ctx.attrs.class === "string" ? ctx.attrs.class as string : "");

  onUpdated(() => {
    klass.value = typeof ctx.attrs.class === "string" ? ctx.attrs.class as string : "";
  });

  return {
    delegate,
    merge,
    tokenize: <R extends readonly Region[]>(...regions: R) => {
      return computed(() => t(klass, ...regions));
    }
  };
}
yankeeinlondon commented 3 years ago

My functionality has been pretty well tested outside of this last test and I've run out of my time budget for now. I really do hope that I'm just being an idiot on how best to use the API but in either case I think it would be really beneficial to others to at least have a good example in the docs if I'm just approaching the API wrong.

Here's all the passing tests; I'll skip testing reactivity for now and maybe try testing it in a consumer library but I'd really like to do this the right way.

image

Unfortunately the code is not open source so there's no easy way for me to share it currently.

lmiller1990 commented 2 years ago

Sorry, this issue completely flew under my radar. setData is mainly for things in the data() function; it sounds like you would like a setAttrs method, but my understanding these are read only and can only change when the data passed.

Is there any way to make a more minimal, concise version of what you'd like to do? There's quite a lot of info in your post, I'm having trouble making a minimal reproduction I can add to our test suite to TDD the problem.

lmiller1990 commented 2 years ago

This issue is pretty old and I don't see any actionable info right now, so I'll close it. If we can have a more concise feature request around attrs, that'd be great - a new issue might be best.