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

[BUG] Vue 3 Component Test Error: "ReferenceError: Item is not defined" #18346

Closed
StepanMynarik opened this issue Oct 26, 2022 · 13 comments
Closed
Assignees

Comments

@StepanMynarik
Copy link

StepanMynarik commented Oct 26, 2022

Context:

  • Playwright Version: 1.27.1
  • Operating System: Windows
  • Node.js version: 16.17.0
  • Browser: All
  • Extra: Vite, Vue 3

playwright test -c playwright-ct.config.ts

Hi,
I have just migrated from Cypress to Playwright and have encountered a similar issue to #17957 in Vue 3.

I'm defining an item type from withing my MultiSelect SFC component like so:

<script lang="ts">
export class Item {
  static defaultHeight = 32;

  key: number | string;
  label: string;
  height: number;
  disabled: boolean | undefined;

  constructor(
    key: number | string,
    label: string,
    height?: number,
    disabled?: boolean
  ) {
    this.key = key;
    this.label = label;
    this.height = height ?? Item.defaultHeight;
    this.disabled = disabled;
  }
}
</script>

Then I just normally reference said type when mocking data for the component test.
This leads to ReferenceError: Item is not defined when running the test.

@sand4rt
Copy link
Collaborator

sand4rt commented Oct 26, 2022

Hi @StepanMynarik, good choice:) could you provide the test and the full component?

@StepanMynarik
Copy link
Author

StepanMynarik commented Oct 26, 2022

Hi @StepanMynarik, good choice:) could you provide the test and the full component?

I have made a full repro project.

You can just:

  • Download repro-project.zip
  • Install Volar extension (official Vue 3 support extension)
  • Enable Take Over mode for workspace (guide is in README.md if needed)
  • 'npm install'
  • 'npm run dev' to see a glorious ItemList.vue component drawing three items
  • Check the 'ItemList.vue' component code
  • Check the 'ItemList.spec.tsx' test code (IDE reports no errors, everything and everyone is happy)
  • Try running the test from previous bullet
  • Observe the error "ReferenceError: Item is not defined"

@sand4rt
Copy link
Collaborator

sand4rt commented Oct 26, 2022

Thanks for the repo and info! You seem to be right about the problem. import { Item } from "./ItemList.vue"; which is imported in itemList.spec.tsx cannot be imported from a component (.vue) file.

I would consider writing the component in a different way or extracting class Item from itemList.vue in its own file like suggested in the docs.

To find out if the component can be written in a different way (which will probably be an improvement of the code anyway), i have to know a little more about what you are trying to solve with this piece of code?:

// itemList.vue
<script lang="ts">
export class Item {
  key: number | string;
  label: string;

  constructor(key: number | string, label: string) {
    this.key = key;
    this.label = label;
  }
}
</script>

<script setup lang="ts">
const { items = [] } = defineProps<{
  items: Item[];
}>();
</script>

<template>
  <div class="vertical-list">
    <div class="item" v-for="item in items" :key="item.key">
      {{ item.label }}
    </div>
  </div>
</template>
// itemList.spec.tsx
import ItemList, { Item } from "./ItemList.vue";
import { expect, test } from "@playwright/experimental-ct-vue";
import type { Locator } from "@playwright/test";
import type { Ref } from "vue";

const initialItemCount = 10;
const mockData = () => {
  const itemsRaw: Item[] = new Array(initialItemCount);
  for (let i = 0; i < initialItemCount; i++) {
    itemsRaw[i] = new Item(i, `Item ${i + 1}`);
  }
  const items = $shallowRef(itemsRaw);
  const selectedItems = $ref([items[1]]);
  return $$({ items, selectedItems });
};

test.describe("<ItemList />", () => {
  let mockedData: {
    items: Ref<Item[]>;
    selectedItems: Ref<Item[]>;
  };
  let component: Locator;

  test.beforeEach(async ({ mount }) => {
    mockedData = mockData();

    component = await mount(
      <ItemList
        items={mockedData.items.value}
      />
    );
  });

  test("draws items", async () => {
    expect(component.locator(".item")).toHaveLength(initialItemCount);
  });
});

@StepanMynarik
Copy link
Author

StepanMynarik commented Oct 27, 2022

Well I like the concept of SFCs.

So when I need to create a highly reusable component like a multiselect for example, I can really use having a base Item class for anyone to use. This class can then have properties like "label", "key", "selected", "disabled" etc. that are used by the base component.

Users of said component can then use just the base Item class, or they can even create an ExtendedItem class for example with extra properties they need in the template they use for MultiSelect's Item slot.
This is extra useful when user needs to customize the multiselect item template with extra fields etc.

Full potential of TypeScript can also be unlocked by making the component generic (new feature of Vue 3) like so <script setup lang="ts" generic="T extends Item">.
Item scoped slot can then provide the user with his custom item type.
At than point, flames erupt from his keyboard and he starts laughing maniacally.

You see my point?

Having the base Item class inside the component makes importing it extra easy. Also keeps project structure lean and clean.
At least for as long as you keep breaking your components into smaller ones :)

This was working in Cypress and was one of few things I liked about it.
Not regretting the switch from Cypress to Playwright, since Playwright is clearly better for my workflow.
Just trying to unlock its full potential in Vue component testing, to not only match Cypress, but to run it into the ground!

@sand4rt
Copy link
Collaborator

sand4rt commented Oct 27, 2022

Thanks for the explanation! I see what you are what you are trying to do here. However IMO having a base class to extends from to create your props and solving this in an object oriented way doesn't seem the right approach to me. I'd just use plain old JavaScript objects instead (think you can also get this working with the new generic RFC as well). This makes the component easier to use and your tests easier to write:

// ItemList.vue
<script lang="ts" setup>
type ItemListProps = {
  items?: {
    key: string | number;
    label: string;
    selected?: boolean;
    [key: string]: any; // any other props
  }[]
}

const { items = [] } = defineProps<ItemListProps>();
</script>

<template>
  <div class="vertical-list">
    <div data-testid="list-item" v-for="item in items" :key="item.key">
      {{ item.label }}
    </div>
  </div>
</template>
// ItemList.spec.tsx
import ItemList from "./ItemList.vue";
import { expect, test } from "@playwright/experimental-ct-vue";

test("render a list of items", async ({ mount }) => {
  const component = await mount(<ItemList items={[
    { key: 1, label: '', anyOtherProp: 2  }, // NOTE: can add additional props
    { key: 2, label: '' }
  ]} />);
  await expect(component.getByTestId("list-item")).toHaveCount(2);
});

@StepanMynarik
Copy link
Author

StepanMynarik commented Oct 27, 2022

Thank you for the suggestion.

I come from the C# world, so I'm very picky about my "restrictive type contracts".
This is a cool technique what you describe, but in my case, I would still prefer class inheritance.
Mainly because by having a specific derived type "injected" into the generic component I can be sure that each and every single item has the extra properties described by the nested class; at every single point in the component.

And when I combine items of multiple types, I make a type guard and am done with it. Again, definitely doable with your technique, but then again. I'm so used to classes and keep switching between languages (for example ASP.NET based backend for my Vue app) so often that it's just better for me to stick with classes I believe.

Srsly, I almost exclusively use classes for everything. It comes naturally to me.

Yes, in JS, you can make check for keys being present on the object. But for example in this case I don't like it much.
I really do prefer having a specific classes, for me it is short and incredibly easy to read and make sense of.

I also believe others would benefit from being able to import types from their vue files. Either from somewhat similar reasons to mine, preference, or because they don't know JS that well (also my case a bit I admit :D)

@sand4rt
Copy link
Collaborator

sand4rt commented Oct 27, 2022

I don't like the classes approach, but yeah many people will run into this problem. Until/if there is a solution these are your only two options:

  1. Don't use a class as prop
  2. Extract the class into it's own file

What do you think of this @pavelfeldman?

@StepanMynarik
Copy link
Author

StepanMynarik commented Oct 27, 2022

Thanks for having patience with me.

I just like to keep my SFCs tight. I even use the custom block for localization. I can just grab any component and copy it into a separate repro/demo/any other project.

Also in combination with <style scoped> block, you will never have useless css nor localization tuples leftovers again.

I also enjoy not having to fish for any component related stuff.
You only have the test tsx file right next to it, which is perfect.

Any tips on stripping the spec.tsx files from the final build?

@pavelfeldman
Copy link
Member

If it's considered a temporary limitation that should be alleviated in the future, then I'm absolutely happy :) I was just reluctant to accept it as an intended permanent state of things.

I would not consider this limitation temporary. As you know, we are running tests in a different environment from the components. We've blurred the boundary as much as we could for better UX, but further blurring it would essentially eliminate that boundary. As a result, the test code that runs in Node will start running the component code that was intended to run within the browser. We will immediately start touching the global object (window), etc.

@StepanMynarik
Copy link
Author

StepanMynarik commented Nov 14, 2022

I'm not that much of a JS world nerd, so I beg your pardon if what I say is BS.

I use classes a lot in my props. And I always keep these short few line classes in my SFC components.
Would it be possible to extract <script lang="ts"> defined exports from within SFCs (*.vue files) without running in browser?
I just need the type. This is my biggest (and one of very few) pain points with Playwright.

I can extract the classes, but then I have to find a nice and sustainable way of organizing them in the project to not break my project structure (which is currently incredibly neat and I never have to think about it, it's incredibly easy to maintain).
It will be a lot of classes and I never put multiple classes in the same file.
It will also require more clicks and cognitive overload to just navigate all the types etc.

@pavelfeldman
Copy link
Member

I just need the type.

If all you need is a type, you should import type { Foo } or import { type Foo }. Types are not a part of the runtime in TypeScript, they are erased during the compilation. It is still respected by the type checkers and IDEs.

@StepanMynarik
Copy link
Author

StepanMynarik commented Nov 15, 2022

I just need the type.

If all you need is a type, you should import type { Foo } or import { type Foo }. Types are not a part of the runtime in TypeScript, they are erased during the compilation. It is still respected by the type checkers and IDEs.

But I can't create an instance of the class from just the type.
This was a bad wording on my side. There are no such differences in C# or any other language I ever used, so still getting used to it.

I would be ok with using an activator like in the bottom answer here for example, but that's not possible when the constructor doesn't actually exist in the test environment at runtime. If I understand it correctly.
Which effectively makes using classes (ctor functions or any other functions) defined in vue/jsx/tsx files impossible :(

I could use interface instead of class, yes, but that isn't the same. You can't implement functions within interfaces, you can't have constructors that post-process passed arguments etc.
Or I could extract class defs into separate files as described in docs, but I just really don't like that idea, since it will needlessly clutter by project structure with lots and lots of small files.

I don't really care that much about some minor bloat within test files, or code cleanliness, so even some workaround would be welcome. If there is one.

@pavelfeldman
Copy link
Member

Why was this issue closed?

Thank you for your contribution to our project. This issue has been closed due to its limited upvotes and recent activity, and insufficient feedback for us to effectively act upon. Our priority is to focus on bugs that reflect higher user engagement and have actionable feedback, to ensure our bug database stays manageable.

Should you feel this closure was in error, please create a new issue and reference this one. We're open to revisiting it given increased support or additional clarity. Your understanding and cooperation are greatly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants