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

🐛 dev server vite 2.2.3 dynamic import are not using path aliases #22390

Closed
hinogi opened this issue Jun 18, 2022 · 28 comments
Closed

🐛 dev server vite 2.2.3 dynamic import are not using path aliases #22390

hinogi opened this issue Jun 18, 2022 · 28 comments
Assignees
Labels
CT Issue related to component testing npm: @cypress/vite-dev-server @cypress/vite-dev-server package issues

Comments

@hinogi
Copy link

hinogi commented Jun 18, 2022

Current behavior

importsToLoad dynamic import of the spec file fails because it does not replace the path alias given by vite in the dynamic import.

const importsToLoad = [specLoader || (() => import(/* @vite-ignore */ specPath))]

This results in a TypeError when it resolves.

Desired behavior

When vite is used and path aliases are given, check all kinds of imports or references to files with the according path alias.

Test code to reproduce

src/components/QuasarButton.vue

<template>
  <input
    data-cy="button"
    label="test emit"
    @click="$emit('test')"
  />
</template>

<script>
import { defineComponent } from 'vue';

export default defineComponent({
  name: 'QuasarButton',
  emits: ['test'],
});

src/components/__test__/QuasarButton.spec.ts

// __test__/QuasarButton.spec.ts

and a path alias for src pointing to src

Cypress Version

10.1.0

Other

The content does not really matter because cypress will not reach any of it because vite has found spec files and is providing the paths with knowing the path alias for it but not resolving it but initCypresssTests is blind to it and will fail trying to dynamically import from the path given by vite.

@lmiller1990 lmiller1990 added component testing npm: @cypress/vite-dev-server @cypress/vite-dev-server package issues labels Jun 22, 2022
@lmiller1990
Copy link
Contributor

Hi, thanks for the bug report. Quasar is super nice to work with.

Just to clarify, you want to do something like:

export default defineConfig({
  plugins: [vue()],
  resolve: {
    alias: {
      '@': 'src'
    }
  }
})

And then type import QuasarButton from '@components/QuasarButton.vue'?

@lmiller1990 lmiller1990 self-assigned this Jun 22, 2022
@cypress-bot cypress-bot bot added the stage: investigating Someone from Cypress is looking into this label Jun 22, 2022
@hinogi
Copy link
Author

hinogi commented Jun 23, 2022

The path alias yes but the spec files are found by cypress itself, it is about spec files

@IlCallo
Copy link
Contributor

IlCallo commented Jun 23, 2022

Adding a bit more context: @hinogi is trying to add support for Cypress 10 into our Cypress App Extension
You can find the draft PR here: quasarframework/quasar-testing#259

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jun 24, 2022

Hello @IlCallo! Long time no talk.

I looked at the draft PR but I think I'm still missing something.

The path alias yes but the spec files are found by cypress itself, it is about spec files

Can you point me some code that doesn't work (or even make a minimal reproduction)? I looked at the draft PR but I do not see any obvious @ alias that isn't working. Also, the initial post doesn't include enough:

src/components/test/QuasarButton.spec.ts
// test/QuasarButton.spec.ts
and a path alias for src pointing to src

What should I do to "and a path alias for src pointing to src"? I think I am missing something.

@lmiller1990 lmiller1990 added stage: awaiting response Potential fix was proposed; awaiting response and removed stage: investigating Someone from Cypress is looking into this labels Jun 24, 2022
@IlCallo
Copy link
Contributor

IlCallo commented Jul 4, 2022

We have a src alias in Quasar and we use it into out component support file to load the user global CSS file as import 'src/css/app.scss';

Seems like Vite isn't able to resolve that alias once we provide the config to Cypress dev server
Since the updated Cypress AE isn't complete and released, it's pretty hard to create the repro, I'll try to create an "edge" release next week so we can create it for you

This is the error we get right now
image

I must say that it seems like tests will continue to load forever anyway even after that has been fixed (eg by using a relative path instead), so we possibly have some other problem

@hinogi
Copy link
Author

hinogi commented Jul 4, 2022

That problem also covers imports in spec files because those paths get used in dynamic imports as is disregarding present path aliases.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jul 11, 2022

I understand now. A minimal reproduction would be very useful for starting debugging.

@lmiller1990
Copy link
Contributor

Seems this is also documented as a Vite problem: vitejs/vite#7611 (comment)
Maybe fixed: vitejs/vite#7756 so it should be fine here, weird?

@IlCallo
Copy link
Contributor

IlCallo commented Jul 13, 2022

From what's written there, they fixed it into Vite 3 branch, but Quasar is still on Vite 2 🤔
But that's strange anyway, since it seemed to work with Cypress 9 and Vite 2

@hinogi
Copy link
Author

hinogi commented Jul 16, 2022

As far as I can see it, the issue is not fixed in vite 3.

@IlCallo
Copy link
Contributor

IlCallo commented Jul 19, 2022

I'm taking a shot with the Cypress 10 migration and I can understand why @hinogi got stuck
I'm pretty sure Vite path aliases actually work and the problem is on our side instead

Quasar did quite some stuff behind the curtains to provide the best DX possible, and this involved using many packages which version has been bumped and are now included into cypress binary (@cypress/vue, @cypress/vite-dev-server, etc)

He forgot to bump those, so that's probably the problem here, but that's understandable: I saw there are A LOT of things we'll need to change/automate for our users during the migration, I barely lost myself too into all of them

I think I almost got to a working repo, after adapting to the new devServer.xxxConfig syntax
Right now I got stuck due to Quasar webpack/vite config being exported as an async function, but Cypress doesn't really seem to like it, so I'm trying to understand if that's the case and I shall revert to export a devServer function instead

quasarframework/quasar-testing@9dc9c43 (#259)

@lmiller1990 any hints?
Some precise questions:

  • do Cypress expect a function into the webpackConfig option or the result of that function? Or both?
  • do Cypress support async execution of cypress.config.ts?
  • do devServer function signature support async functions?

If you don't know right away, no problem, I can somehow dig into Cypress code and find it out by myself
I probably won't be able to complete and release the new version before mid-August anyway, since I'm going to be on vacation in a few days 😅

@nagash77 nagash77 assigned marktnoonan and unassigned lmiller1990 Jul 19, 2022
@cypress-bot cypress-bot bot added stage: investigating Someone from Cypress is looking into this and removed stage: awaiting response Potential fix was proposed; awaiting response labels Jul 21, 2022
@lmiller1990
Copy link
Contributor

do Cypress expect a function into the webpackConfig option or the result of that function? Or both?

Looks like you should pass in an object, so the result: https://github.com/cypress-io/cypress/blob/develop/system-tests/project-fixtures/next/cypress.config.js#L9-L14

do Cypress support async execution of cypress.config.ts?

Good question, I actually have no idea, but why would you want to declare cypress.config.ts to be async? I might be missing something here.

do devServer function signature support async functions?

I think this should work just fine. Eg:

import { devServer } from '@cypress/webpack-dev-server'

export default defineConfig({
  component: {
    devServer: async (devServerOptions) => devServer({
      ...devServerOptions,
      framework: 'vue',
    }),
  }
})

If any of these things don't work how you expect, we can definitely and quickly make non-breaking changes to support frameworks like Quasar. Making it easy to add Cypress to frameworks is important.

@cypress-bot cypress-bot bot added stage: awaiting response Potential fix was proposed; awaiting response and removed stage: investigating Someone from Cypress is looking into this labels Jul 26, 2022
@cypress-bot cypress-bot bot added stage: backlog and removed stage: awaiting response Potential fix was proposed; awaiting response labels Aug 2, 2022
@IlCallo
Copy link
Contributor

IlCallo commented Aug 8, 2022

Good question, I actually have no idea, but why would you want to declare cypress.config.ts to be async? I might be missing something here.

We extract the webpack and Vite config from Quasar, but to detect which bundler we're using we need to import the package.json via an async call
We then do another async call to the package itself (@quasar/app-webpack or @quasar/app-vite) to generate and extract the config
We aren't able to get it in a synchronous way, so the only other option would be to go for the devServer route, which is a bit more complex

Would it be too much of an effort to support an async config for cypress, or at least to support a promise for webpackConfig/viteConfig options?
It would simplify the setup for us as we'd be able to do this quasarframework/quasar-testing@9dc9c43 (#259)

@hinogi
Copy link
Author

hinogi commented Aug 8, 2022

I made some progress with the suggestions of @lmiller1990 This seems to work so far, now @IlCallo has to figure out some stuff for me I guess :D

import { defineConfig } from 'cypress';
import { quasarWebpackConfig } from '@quasar/quasar-app-extension-testing-e2e-cypress/cct-dev-server';
import { devServer } from '@cypress/webpack-dev-server';


export default defineConfig({
  fixturesFolder: 'test/cypress/fixtures',
  screenshotsFolder: 'test/cypress/screenshots',
  videosFolder: 'test/cypress/videos',
  video: true,
  e2e: {
    baseUrl: 'http://localhost:8080/',
    specPattern: 'test/cypress/integration/**/*.cy.{js,jsx,ts,tsx}',
    supportFile: 'test/cypress/support/e2e.ts',
  },
  component: {
    supportFile: 'test/cypress/support/component.ts',
    specPattern: 'src/**/*.spec.{js,jsx,ts,tsx}',
    devServer: async (devServerOptions) => {
      return devServer({
        ...devServerOptions,
        framework: 'vue',
        webpackConfig: await quasarWebpackConfig(),
      });
    },
    indexHtmlFile: 'test/cypress/support/component-index.html',
  },
});

@lmiller1990
Copy link
Contributor

Nice, lmk if you need anything on my end.

@IlCallo
Copy link
Contributor

IlCallo commented Aug 9, 2022

Would it be too much of an effort to support an async config for cypress, or at least to support a promise for webpackConfig/viteConfig options?

This would help us, as we wouldn't need to add @cypress/webpack-dev-server/@cypress/vite-dev-server dependencies ourselves
Anyway we'll try to move on and release an alpha using devServer option instead

@lmiller1990
Copy link
Contributor

lmiller1990 commented Aug 11, 2022

Would it be too much of an effort to support an async config for cypress, or at least to support a promise for webpackConfig/viteConfig options?

I am not sure about an async config for Cypress. We can definitely do an async function for webpackConfig and viteConfig. Maybe we can do:

defineConfig({
  component: {
    devServer: {
      viteConfig: async () => {
        const newConfig = await something()
        return newConfig
      })
    }
  }
})

Would this be useful @IlCallo @hinogi?

We could also include an argument to the async function, that would be our base config, potentially, if that would be useful. So it would be:

viteConfig: async (baseConfig) => {
  const overrides = await something()
  return {...baseConfig, ...overrides }
})

@hinogi
Copy link
Author

hinogi commented Aug 11, 2022

viteConfig: async (baseConfig) => {
  const overrides = await something()
  return {...baseConfig, ...overrides }
})

That was pretty much my first attempt, without async/await, with the then quasar implementation.

@IlCallo
Copy link
Contributor

IlCallo commented Aug 11, 2022

Yep, that would be pretty useful as we wouldn't have to manage @cypress/webpack-dev-server/@cypress/vite-dev-server ourselves 😁

At that point we would just expose 2 distinct functions returing a promise for webpack/vite config and setting up Cypress would be cleaner and easier for the user :)

@lmiller1990
Copy link
Contributor

Ok, will look at doing this one. I will track it here: #23302

@lmiller1990 lmiller1990 added CT Issue related to component testing and removed component testing labels Aug 15, 2022
@IlCallo
Copy link
Contributor

IlCallo commented Aug 16, 2022

I just tried the devServer route, but apparently that's not viable as well, at least not for TS projects

image
From what I understood, @cypress/webpack-dev-server is meant to be bundled with Cypress, but it isn't re-exporting the types

At the same time, if I try to install @cypress/webpack-dev-server as a standalone package I get errors about html-webpack-plugin not being present. This seems due to the package renaming it to have both a webpack v4 and v5 dependency in the same project to support both, but Quasar then cannot resolve it of course as the name isn't the one it expects

@lmiller1990
Copy link
Contributor

lmiller1990 commented Aug 16, 2022

@cypress/webpack-dev-server should be usable in a stand-alone fashion, too, I know some people are using it like this. I am not sure if they just hacked around the types.

Some info on html-plugin-webpack situation: #23334

We are trying to resolve this, it's a little messy, if you want to hack into this more than happy to facilitate a fast review and release! Don't be hesitant to make a change if you need it, if CI is failing we can pick things up internally to help get it over the line.

@IlCallo
Copy link
Contributor

IlCallo commented Aug 17, 2022

I think the problem is that Quasar needs html-plugin-webpack to work, but apparently @cypress/webpack-dev-server hijacks the dependency and change its name, making Quasar unable to find it at runtime
My supposition is that @cypress/webpack-dev-server is surely usable as standalone package, until the project actually requires html-plugin-webpack to work, and the problem appears only with some package managers (yarn 1 for sure)

Unluckily we won't be able to make Quasar "yarn 2-friendly" and drop yarn 1 support before Quasar 3 major version, which will probably happen late next year 😅

Not sure I have the bandwidth to work on it right now, but I'll try as soon as I free up a bit

@lmiller1990
Copy link
Contributor

Going to carry on discussion in #23334

@lmiller1990
Copy link
Contributor

I am working on something in this area: #23605

@lmiller1990
Copy link
Contributor

#23605 landed, it'll be in the next release, I hope this helps!

We can keep working on this, I want to enable third parties as much as possible.

@IlCallo
Copy link
Contributor

IlCallo commented Sep 22, 2022

Nice, we'll try this out as soon as new version is released!

@IlCallo
Copy link
Contributor

IlCallo commented Oct 11, 2022

I'm back at it 😁
Seems like the support for async webpack config did the trick, so this issue can be closed
We're now dealing with another problem, but I'll open a dedicated issue about it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CT Issue related to component testing npm: @cypress/vite-dev-server @cypress/vite-dev-server package issues
Projects
None yet
Development

No branches or pull requests

5 participants