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

fix(vite-dev-server): replace UserConfig with InlineConfig to allow correct configFile types #18167

Merged
merged 5 commits into from
Sep 24, 2021

Conversation

mrmartineau
Copy link
Contributor

  • Closes n/a

User facing changelog

Fixes a TypeScript error when specifying the configFile option for @cypress/vite-dev-server.

Additional details

When using the configFile option in @cypress/vite-dev-server I recieved a TS error. This is because the TS types for the startDevServer method use UserConfig from Vite, but that type does not include the configFile property, however InlineConfig does because it extends from UserConfig.

I replaced references of UserConfig with InlineConfig.

FYI, the TS error message was:

Type '{ configFile: string; }' is not assignable to type 'UserConfig'.\n  Object literal may only specify known properties, and 'configFile' does not exist in type 'UserConfig'.

How has the user experience changed?

There is no change to the user experience.

PR Tasks

  • Have tests been added/updated?
  • Has the original issue or this PR been tagged with a release in ZenHub?
  • Has a PR for user-facing changes been opened in cypress-documentation?
  • Have API changes been updated in the type definitions?
  • Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 20, 2021

Thanks for taking the time to open a PR!

@CLAassistant
Copy link

CLAassistant commented Sep 20, 2021

CLA assistant check
All committers have signed the CLA.

@mrmartineau mrmartineau changed the title replace UserConfig with InlineConfig to allow correct configFile types fix: replace UserConfig with InlineConfig to allow correct configFile types Sep 21, 2021
@mrmartineau
Copy link
Contributor Author

Can any maintainers help me to fix the build issues. Each time I update my branch to be up-to-date with the base different tests fail.. I only changed a TypeScript type and it won't have affected any of the failing tests..

@jennifer-shehane jennifer-shehane changed the base branch from develop to master September 21, 2021 14:44
@jennifer-shehane jennifer-shehane requested a review from a team as a code owner September 21, 2021 14:44
@jennifer-shehane jennifer-shehane requested review from flotwig and chrisbreiding and removed request for a team September 21, 2021 14:44
@jennifer-shehane jennifer-shehane changed the base branch from master to develop September 21, 2021 14:44
@jennifer-shehane jennifer-shehane requested review from a team and removed request for flotwig and chrisbreiding September 21, 2021 14:44
@elevatebart
Copy link
Contributor

@mrmartineau I believe we merged something a bit quickly and there is a need to update yarn.lock for the vue tests to pass.
I updated your branch with the last commit, it should make it work.

Copy link
Contributor

@elevatebart elevatebart left a comment

Choose a reason for hiding this comment

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

Good catch thank you

@lmiller1990
Copy link
Contributor

lmiller1990 commented Sep 24, 2021

Hm, I wonder why npm/vue is now failing. It's passing fine on my local machine.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Sep 24, 2021

Ok, it passes now.

@lmiller1990 lmiller1990 changed the title fix: replace UserConfig with InlineConfig to allow correct configFile types fix(vite-dev-server): replace UserConfig with InlineConfig to allow correct configFile types Sep 24, 2021
@lmiller1990 lmiller1990 merged commit 6e0c2c1 into cypress-io:develop Sep 24, 2021
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.

4 participants