breaking: add support for ressources with relative paths #26844

fochlac · 2023-05-23T21:43:12Z

Closes 26725

Additional details

Steps to test

create a cypress test that mounts a static ressource via a relative path, i.e. <img src="./path/to/img.png" /> and check whether the request succeds

How has the user experience changed?

It is now possible to distinguish the initial request for the index.html of the CT from relative asset requests (fonts/images/etc). This allows routing those requests to dev-server while retaining the relative paths.
Example: if a component test includes an image defined as follows: <img src="./path/to/img.png" /> the load image request will be forwarded to the url dev-server:port/__cypress/src/path/to/img.png instead of dev-server:port/__cypress/src/index.html.

PR Tasks

Have tests been added/updated?
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?

CLAassistant · 2023-05-23T21:43:17Z

All committers have signed the CLA.

cypress-app-bot · 2023-05-23T21:43:32Z

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.

cypress · 2023-05-23T22:56:17Z

Passing run #47021 ↗︎

0	104	0	0	0

Details:

Merge branch 'develop' into issues-26725-fix-relative-ressource-requests-in-ct
Project: cypress	Commit: `8101000f0e`
Status: Passed	Duration: 16:16 💡
Started: Jun 5, 2023 8:22 AM	Ended: Jun 5, 2023 8:38 AM

_{This comment has been generated by cypress-bot as a result of this
project's GitHub integration settings.}

system-tests/lib/dep-installer/index.ts

system-tests/test/webpack_dev_server_spec.ts

lmiller1990 · 2023-06-05T08:16:45Z

npm/webpack-dev-server/src/loader.ts

@@ -18,7 +18,7 @@ const makeImport = (file: Cypress.Cypress['spec'], filename: string, chunkName:
  const magicComments = chunkName ? `/* webpackChunkName: "${chunkName}" */` : ''

  return `"${filename}": {
-    shouldLoad: () => decodeURI(document.location.pathname).includes("${file.absolute}"),
+    shouldLoad: () => (new URL(document.location)).searchParams.get("specPath") === "${file.absolute}",


I think this should be fine, let's see how CI goes.

It works, however, this makes it a breaking change I think. This will become a problem if the cypress-version doesn't match the webpack-dev-server version, and I don't know if I can assume this to always be the case.
I considered supporting both variants to be backward compatible, but was not sure if its actually needed.

Why is this a breaking change? Can you share some examples of when shouldLoad will be return a different value after your change?

Making a breaking change is definitely not something we take lightly, I think we want to avoid this if possible. Otherwise, this change would need to wait for the next major - a lot of planning goes into these.

If this change would be part of 12.13.1 and some user uses cypress@12.13.1 and @cypress/webpack-dev-server@12.13.0 (or the other way round) no tests would load.

Right, gotcha. Actually this won't be a big problem, but it will be breaking.

We'd wait for Cypress 13, and update this table (from the npm module readme: https://www.npmjs.com/package/@cypress/webpack-dev-server)

Let me review and test this a bit more. This will need to be part of v13.

I'd missed that document.pathname does NOT include search params. I understand the breaking change now.

I extended the table, should I also update the changelog?

The changelog for npm is automated, no problem here.

lmiller1990

This looks pretty good, let me sync with the team on how to get this in v13.

lmiller1990 · 2023-06-07T05:00:42Z

@fochlac I talked to the team, this will be able to go in Cypress 13. That is at least a month away, so this won't merge for a while, but I'll get some more reviewers on it for you, and we can communicate as Cypress 13 draws closer.

For reference and for other reviewers:

What’s the breaking change that users would experience?

If a user has installed @cypress/webpack-dev-server from npm and configured their own dev-server, then upgraded to a version of Cypress with this change, it would break (the spec would not load).

This workflow (install @cypress/webpack-dev-server manually, instead of using the one that we ship with the binary) is pretty uncommon, it’ll only impact a handful of power users. The change here is an implementation detail. It’s only breaking because the contract between our npm module @cypress/webpack-dev-server and the binary is not guaranteed if you install @cypress/webpack-dev-server on your own instead of the one we include in the binary.

What docs would need to be changed?

Nothing in our main docs, we’d just update this table. This is actually an implementation detail, not part of the public API.

fochlac · 2023-06-07T12:37:28Z

@lmiller1990 sounds great, waiting for Cypress 13 is no big deal. Thanks for summarizing the breaking changes. I guess I was too deep into the topic to see why it's not obvious. :)

lmiller1990 · 2023-06-07T22:29:44Z

@fochlac thought about this one a bit more - what do you think about keeping the old conditional, and including the new one with an or statement? No breaking change that way - it'll minimize any risk, and we could even release it in a minor/patch.

fochlac · 2023-06-07T22:36:45Z

@lmiller1990 I thought about this as well, but I think it just covers a less common case.
Let's have a look at the options (considering this is released with 13.0.0):

@cypress/webpack-dev-server@12.13.0 and cypress@13.0.0 -> tests will not load, as the test-path is in the query params and dev-server checks pathname
@cypress/webpack-dev-server@13.0.0 and cypress@12.13.0 -> tests will not load, as test-path is in pathname but webpack checks query params

That change would prevent case 2, however I think that one is the one that is much less likely to occur, that's why I have not left in the alternative check. If you think it's worth it, I'm open to add it though.

lmiller1990 · 2023-06-07T23:01:19Z

We've got a few weeks to think about it either way. The cleanest way is likely the breaking change as you are pointing out, but glad we considered both angles.

We could potentially add a check in Cypress 13 that looks for an older version of @cypress/webpack-dev-server and throws an error such as "You are on Cypress 13, but using an old version of @cypress/webpack-dev-server. Please update `@cypress/webpack-dev-server`` to version 4".

It would be v4 since the current one is v3: https://www.npmjs.com/package/@cypress/webpack-dev-server - the Cypress major isn't tied to the @cypress/webpack-dev-server version.

Just to clarify, we ship the correct version of @cypress/webpack-dev-server in the binary as part of the cypress package - only users who install @cypress/webpack-dev-server manually (who might want to override the default behavior) will be impacted by this - anyone using the defaults will automatically get updated when they install Cypress 13, so this shouldn't be a problem for the majority of users.

lmiller1990 · 2023-06-20T22:59:57Z

Hey @fochlac, we might want to rebase and retarget this to the v13 branch: #27040

fochlac · 2023-06-21T20:28:54Z

Thanks for the head's up. I rebased and retargeted the PR. I also adapted the table you mentioned. From your PoV what are the remaining tasks I need to do for this PR?

lmiller1990 · 2023-06-22T00:39:33Z

I don't think any additional work is needed on your end. Let me run CI and see if this is still green.

lmiller1990 · 2023-07-19T20:26:02Z

Cy 13 coming soon. We will get this ready go to.

lmiller1990 · 2023-07-19T23:48:47Z

npm/webpack-dev-server/README.md

@@ -68,6 +68,7 @@ We then merge the sourced config with the user's webpack config, and layer on ou
 | --------------------------- | ------- |
 | <= v1                       | <= v9   |
 | >= v2                       | >= v10  |
+| >= v4                       | >= v13  |


@fochlac hey! We are looking to merge this soon. Should this be v3?

Also can you rebase again? Thank you!

If you can't, let me know and I can try to pick it up.

Hi, only saw your comment just now. I think the new version should be v4 as we are currently on v3, and for full compatibility we need dev-server >=v4 and cypress >=v13.
We could consider adapting this like this as well for more clarity:

@cypress/webpack-dev-server cypress

<= v1 <= v9

v2 - v3 v10 - v12

>= v4 >= v13

lmiller1990 · 2023-08-02T01:37:14Z

I resolved the conflicts, let's get this green and ready to merge.

…e-requests-in-ct

lmiller1990 · 2023-08-10T00:02:12Z

CI has some issues w/ third party PRs right now. I will try re-opening a new PR with the same commits: #27510

lmiller1990 · 2023-08-14T09:26:05Z

Closing for #27510

Same code but this one cannot pass CI for unknown reasons

fochlac force-pushed the issues-26725-fix-relative-ressource-requests-in-ct branch from d1ea71c to 67b8730 Compare May 23, 2023 21:52

fochlac marked this pull request as ready for review May 23, 2023 22:23

fochlac marked this pull request as draft May 23, 2023 22:25

fochlac force-pushed the issues-26725-fix-relative-ressource-requests-in-ct branch from 4ed7300 to 5e4d02e Compare June 4, 2023 23:36

fochlac marked this pull request as ready for review June 4, 2023 23:38

lmiller1990 requested a review from a team June 5, 2023 03:36

lmiller1990 reviewed Jun 5, 2023

View reviewed changes

lmiller1990 reviewed Jun 6, 2023

View reviewed changes

nagash77 assigned lmiller1990 Jun 7, 2023

fochlac changed the base branch from develop to release/13.0.0 June 21, 2023 19:51

fochlac added 2 commits June 21, 2023 22:02

fix: support ressources with relative paths

01f785e

fix: support ressources with relative paths - review changes

0013774

fochlac force-pushed the issues-26725-fix-relative-ressource-requests-in-ct branch from 350dafd to 0013774 Compare June 21, 2023 20:10

fochlac changed the title ~~fix: support ressources with relative paths~~ breaking: add support for ressources with relative paths Jun 21, 2023

lmiller1990 reviewed Jul 19, 2023

View reviewed changes

lmiller1990 requested a review from a team July 19, 2023 23:49

resolve conflicts

82f2ba4

astone123 and others added 8 commits August 8, 2023 12:28

Merge branch 'release/13.0.0' into issues-26725-fix-relative-ressourc…

27e64da

…e-requests-in-ct

try fixing contributor workflow

f452caf

try to fix regex

f8adf1c

try again

6450dff

try workflow again

f55e63a

Revert changes to workflow

3c22476

Merge branch 'release/13.0.0' into issues-26725-fix-relative-ressourc…

e39838b

…e-requests-in-ct

exclude tests from contributor workflow

d8900f3

lmiller1990 mentioned this pull request Aug 10, 2023

breaking: relative resource requests in ct #27510

Merged

jordanpowell88 mentioned this pull request Aug 11, 2023

Angular Component Test: Load asset for Image not working #27529

Closed

lmiller1990 closed this Aug 14, 2023

marktnoonan mentioned this pull request Oct 11, 2023

Component tests break due to Cypress version 13 adding a 'specPath' query parameter to the URL #28021

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

breaking: add support for ressources with relative paths #26844

breaking: add support for ressources with relative paths #26844

fochlac commented May 23, 2023 •

edited

Loading

CLAassistant commented May 23, 2023 •

edited

Loading

cypress-app-bot commented May 23, 2023

cypress bot commented May 23, 2023 •

edited

Loading

lmiller1990 Jun 5, 2023

fochlac Jun 5, 2023 •

edited

Loading

lmiller1990 Jun 6, 2023

fochlac Jun 6, 2023

lmiller1990 Jun 6, 2023 •

edited

Loading

lmiller1990 Jun 6, 2023

fochlac Jun 21, 2023

lmiller1990 Jul 19, 2023

lmiller1990 left a comment

lmiller1990 commented Jun 7, 2023 •

edited

Loading

fochlac commented Jun 7, 2023

lmiller1990 commented Jun 7, 2023

fochlac commented Jun 7, 2023 •

edited

Loading

lmiller1990 commented Jun 7, 2023 •

edited

Loading

lmiller1990 commented Jun 20, 2023

fochlac commented Jun 21, 2023

lmiller1990 commented Jun 22, 2023

lmiller1990 commented Jul 19, 2023

lmiller1990 Jul 19, 2023

fochlac Aug 8, 2023 •

edited

Loading

lmiller1990 commented Aug 2, 2023

lmiller1990 commented Aug 10, 2023

lmiller1990 commented Aug 14, 2023

breaking: add support for ressources with relative paths #26844

breaking: add support for ressources with relative paths #26844

Conversation

fochlac commented May 23, 2023 • edited Loading

Additional details

Steps to test

How has the user experience changed?

PR Tasks

CLAassistant commented May 23, 2023 • edited Loading

cypress-app-bot commented May 23, 2023

cypress bot commented May 23, 2023 • edited Loading

Passing run #47021 ↗︎

lmiller1990 Jun 5, 2023

Choose a reason for hiding this comment

fochlac Jun 5, 2023 • edited Loading

Choose a reason for hiding this comment

lmiller1990 Jun 6, 2023

Choose a reason for hiding this comment

fochlac Jun 6, 2023

Choose a reason for hiding this comment

lmiller1990 Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

lmiller1990 Jun 6, 2023

Choose a reason for hiding this comment

fochlac Jun 21, 2023

Choose a reason for hiding this comment

lmiller1990 Jul 19, 2023

Choose a reason for hiding this comment

lmiller1990 left a comment

Choose a reason for hiding this comment

lmiller1990 commented Jun 7, 2023 • edited Loading

fochlac commented Jun 7, 2023

lmiller1990 commented Jun 7, 2023

fochlac commented Jun 7, 2023 • edited Loading

lmiller1990 commented Jun 7, 2023 • edited Loading

lmiller1990 commented Jun 20, 2023

fochlac commented Jun 21, 2023

lmiller1990 commented Jun 22, 2023

lmiller1990 commented Jul 19, 2023

lmiller1990 Jul 19, 2023

Choose a reason for hiding this comment

fochlac Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

lmiller1990 commented Aug 2, 2023

lmiller1990 commented Aug 10, 2023

lmiller1990 commented Aug 14, 2023

fochlac commented May 23, 2023 •

edited

Loading

CLAassistant commented May 23, 2023 •

edited

Loading

cypress bot commented May 23, 2023 •

edited

Loading

fochlac Jun 5, 2023 •

edited

Loading

lmiller1990 Jun 6, 2023 •

edited

Loading

lmiller1990 commented Jun 7, 2023 •

edited

Loading

fochlac commented Jun 7, 2023 •

edited

Loading

lmiller1990 commented Jun 7, 2023 •

edited

Loading

fochlac Aug 8, 2023 •

edited

Loading