cypress
cypress copied to clipboard
fix: typo in viewport dropdown
- Closes #23632
User facing changelog
Additional details
Steps to test
How has the user experience changed?
PR Tasks
- [ ] Have tests been added/updated?
- [ ] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
- [ ] Has a PR for user-facing changes been opened in
cypress-documentation? - [ ] Have API changes been updated in the
type definitions?
Thanks for taking the time to open a PR!
- Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
- Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.
** Ignore this comment! ** commit https://github.com/cypress-io/cypress/commit/4b09dc3b781cf5bc0baef85ab3b4c4cef1872444 solves the issue.
I was unable to resolve the issue locally so I have not created a PR. I took some notes while testing / exploring and I will include them here.
Files
packages/frontend-shared/src/locales/en-US.jsonpackages/app/src/runner/SpecRunnerHeaderOpenMode.vuepackages/app/src/runner/SpecRunnerHeaderOpenMode.cy.tsxpackages/app/cypress/e2e/cypress-in-cypress-e2e.cy.ts
Origin
The issue seems to originate within the function responsible for
parsing and replacing in-string placeholder values with real values.
This text has string-vars like: in string var like {this} replaced.
The original (unreplaced) text segment exists in a locale source (file-1)
at runner.viewportTooltip.configText
The adjacent entry of runner.viewportTooltip.infoText has its
placeholder values correctly replaced at {0}, {1} and {2}
The locale object values are passed to an <i18n-t> component as the
keypath prop in (file-2)
<i18n-t
tag="p"
keypath="runner.viewportTooltip.configText"
class="mb-24px"
>
The test file (file-3) contains a test at line 196 that checks for the existence and visibility of the correctly replaced string:
cy.contains('Additionally, you can override this value in your cypress.config.js or via the cy.viewport() command.')
^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^
.should('be.visible')
There is also an example cypress test in (file-4) that tests for the
adjacent config text runner.viewportTooltip.infoText but, does not
test for runner.viewportTooltip.configText
@FireNdIce3 Do you think you'll be able to add a test for this?
@chrisbreiding Interestingly our Component tests for this are now failing with this fixed behavior which was testing it originally https://app.circleci.com/pipelines/github/cypress-io/cypress/43400/workflows/3eb83d3f-5f15-496c-a78c-d9e62e697373/jobs/1811601
- https://github.com/cypress-io/cypress/blob/develop/packages/app/src/runner/SpecRunnerHeaderOpenMode.cy.tsx#L184
- https://github.com/cypress-io/cypress/blob/develop/packages/app/src/runner/SpecRunnerHeaderOpenMode.cy.tsx#L207
Wonder why these are inconsistent 🤔
Interestingly our Component tests for this are now failing with this fixed behavior which was testing it originally app.circleci.com/pipelines/github/cypress-io/cypress/43400/workflows/3eb83d3f-5f15-496c-a78c-d9e62e697373/jobs/1811601
[develop/packages/app/src/runner/SpecRunnerHeaderOpenMode.cy.tsx#L184](https://github.com/cypress-io/cypress/blob/develop/packages/app/src/runner/SpecRunnerHeaderOpenMode.cy.tsx?rgh-link-date=2022-09-20T18%3A31%3A16Z#L184) [develop/packages/app/src/runner/SpecRunnerHeaderOpenMode.cy.tsx#L207](https://github.com/cypress-io/cypress/blob/develop/packages/app/src/runner/SpecRunnerHeaderOpenMode.cy.tsx?rgh-link-date=2022-09-20T18%3A31%3A16Z#L207)Wonder why these are inconsistent 🤔
@emilyrohrbough Took a little digging, but I figured this out and figured out the root cause of the original bug.
The <i18n-t> component can do string interpolation in two ways. The way we were using it (and still using it in other places) relied on list interpolation, where {0}, {1}, etc rely on the order and placement of the children nested within the <i18n-t> component.
The original code looks like this (with children indexes noted):
<i18n-t ...>
0: <!-- disable rule to prevent trailing space from being added to <InlineCodeFragment/> -->
1: <!-- eslint-disable-next-line vue/singleline-html-element-content-newline -->
2: <InlineCodeFragment class="font-medium text-xs leading-5">{{ props.gql.configFile }}</InlineCodeFragment>
3: <!-- eslint-disable-next-line vue/singleline-html-element-content-newline -->
4: <InlineCodeFragment class="font-medium text-xs leading-5">cy.viewport()</InlineCodeFragment>
</i18n-t>
So we used the interpolated values {2} and {4}. This works locally, in development, and in CI.
However, I'm guessing that when we build for production and produce the binary, the comments are stripped out before it does the i18n replacement, so the Vue markup ends up looking like this:
<i18n-t ...>
0: <InlineCodeFragment class="font-medium text-xs leading-5">{{ props.gql.configFile }}</InlineCodeFragment>
1: <InlineCodeFragment class="font-medium text-xs leading-5">cy.viewport()</InlineCodeFragment>
</i18n-t>
So then the interpolated values should be {0} and {1}. {2} and {4} don't even exist, so it ends up putting in empty strings (it would be nice if the build failed in that case, eh?). That's why the component tests used to pass but failed when using {0} and {1}. But {0} and {1} would have worked in the production app.
To resolve this, I changed the markup to use the other string interpolation method, slots syntax. It ensures the same values are interpolated regardless of child order or the presence/lack-of-presence of comments.
@chrisbreiding whoa....nice find! I bet you are correct that prod mode was stripping these out.