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

Add runtime semver checks ensuring the version of vite / other external dependencies #22025

Closed
sync-by-unito bot opened this issue Jun 2, 2022 · 5 comments · Fixed by #22964
Closed
Assignees
Labels
CT Issue related to component testing

Comments

@sync-by-unito
Copy link

sync-by-unito bot commented Jun 2, 2022

Summary

We want to notify the user if they are potentially using an older/unsupported external dependencies of vite, next, nuxt, etc with a runtime warning. We currently only do the dependencies check during onboarding, but we should be doing it whenever they are starting up the app with a semver check.

Acceptance Criteria

  • Should verify accepted semver ranges for external dependencies
  • Should validate that the user’s dependencies falls in that range
    • If it does not, display a warning to the user
  • Determine other packages that we need to do this for & add these checks

Resources

Any Notion documents, Google documents, Figma Boards

┆Issue is synchronized with this Jira Task by Unito
┆author: Tim Griesser
┆friendlyId: UNIFY-1818
┆priority: None
┆sprint: Fast Follows 1
┆taskType: Task

@sync-by-unito sync-by-unito bot added CT Issue related to component testing fast-follows-1 labels Jun 2, 2022
@rockhold
Copy link

There are some unknowns around this issue that should be explored before being picked up. We could probably use some more info from @tgriesser on his thoughts on how to address this, and then we'll potentially need product sign-off as well. It doesn't seem likely that we'll be able to address this sprint.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Jul 25, 2022

@rockhold

There are some unknowns around this issue that should be explored before being picked up.
...
potentially need product sign-off as well

What unknowns do you see?

The only question I have is how the runtime warnings manifests... this is the main unknown, I'm not sure how best to handle this - how about a console.warn in the terminal? I wonder if this even needs to manifest in the UI.

We have a pattern that might be of use. We could show the warnings here. Proposed this here, waiting on response.

image

Here's how it might look:

image

@mapsandapps
Copy link
Contributor

Results of design meeting, July 26, cc @lmiller1990 :

"Try again" probably doesn't work since you would need to restart Cypress for it to get the new dependencies. If it works, we can keep the try again button, otherwise, we should remove the button.

The content should be clearer about what is going on (this is basically just a recommendation; we don't know if anything will really break or not. And that's not very clear from the current text.)

With those changes, we can move ahead with this design. @christopherfrance can help with the details on the above two points.

@lmiller1990
Copy link
Contributor

Thanks @mapsandapps. We can get started on the implementation, and get the final wording in an async fashion from @christopherfrance.

Agreed on removing the "Try Again" button.

@cbfrance
Copy link

Thanks @lmiller1990 this looks like a good improvement from design perspective.

There are two changes being discussed in this thread:

  • DONE: Remove Restart button since it seems unlikely to work as expected.

  • TODO: The last line could more actionable / explain what to do next. Here is our suggestion (to
    replace Cypress may not function as expected) — If you're experiencing problems, downgrade dependencies and restart Cypress.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants