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

Error with assets loading from scss in Angular #24272

Closed
theoarav opened this issue Oct 17, 2022 · 14 comments · Fixed by #25191
Closed

Error with assets loading from scss in Angular #24272

theoarav opened this issue Oct 17, 2022 · 14 comments · Fixed by #25191
Assignees
Labels
CT Issue related to component testing

Comments

@theoarav
Copy link

Current behavior

Loading assets from scss with something like this :

<div class="testClass"></div>
.testClass {
  background-image: url("/assets/test.png");
  background-size: contain;
  width: 100px;
  height: 100px;
}

Image is showing using ng serve but not when using cypress component testing

If I remove the '/' in the url, image load in component testing but angular won't compile with the following error :

./src/app/app.component.scss?ngResource - Error: Module Error (from ./node_modules/postcss-loader/dist/cjs.js):
[...]/theTester/src/app/app.component.scss:2:2: Can't resolve './assets/test.png' in '[...]/theTester/src/app'

Another loading problem comes from loading assets in scss "main" files, I have a main.scss containing :

@import "components/icon";

For simplicity purposes I didn't use abstracts variables and mixins.

components/icons :

$imagesTest: test "/assets/test.png", testSecond "/assets/test.png";

.icon {
  top: 0px;
  position: relative;

  &.yolo {
    background-image: url("/assets/test.png");
    background-size: contain;
    background-repeat: no-repeat;
  }

  @each $class, $url in $imagesTest {
    &.#{$class} {
      background-image: url($url);
      background-size: contain;
      background-repeat: no-repeat;
    }
  }
}

app.component.html:

  ICON TEST:
  <div
    class="icon test"
    style="display: block; width: 50px; height: 50px"
  ></div>
  YOLO:
  <div
    class="icon yolo"
    style="display: block; width: 50px; height: 50px"
  ></div>

If I remove the / here, images won't load from assets since it's using relative path and it will try to load assets/components/test.png and ng serve won't compile with the same error above.

Desired behavior

I would like to be able to load all my assets, I've given a sample app reproducing the issue below, but in my real-world app, I can't load any of my svg that comes from scss and all the fonts I use aren't loaded since I'm using @font-face with urls in scss and they can't be resolved.

Test code to reproduce

Repo with code: https://github.com/theoarmengou/angular-cypress-bug-assets

When doing ng serve, you should see 4 images on localhost:4200 and when going to localhost:4200/dev, there should be 8 images.

When using yarn cypress:open in component testing, not all images are showing for app.component and dev.component.

Cypress Version

10.10.0

Node version

16.16.0

Operating System

macOS 12.5.1

Debug Logs

No response

Other

No response

@warrensplayer
Copy link
Contributor

What is working for me is to use relative paths for the images in the SCSS files.

So app.component.scss looks like:

.testClass {
  background-image: url("../assets/test.png");
  background-size: contain;
  width: 100px;
  height: 100px;
}

And _icon.scss looks like:

$imagesTest: test "../../assets/test.png", testSecond "../../assets/test.png";

.icon {
  top: 0px;
  position: relative;

  &.yolo {
    background-image: url("../../assets/test.png");
    background-size: contain;
    background-repeat: no-repeat;
  }

  @debug $images;

  @each $class, $url in $imagesTest {
    &.#{$class} {
      background-image: url($url);
      background-size: contain;
      background-repeat: no-repeat;
    }
  }
}

See if those changes work for you as well.

@theoarav
Copy link
Author

I'm pretty sure it would work for me too, but when you have nested components that load assets from scss it's pretty bad to have relative links instead of absolute.

Having a link looking like this ../../../../../../assets/test.png isn't as pretty as /assets/test.png...

And if the asset gets moved or the component is moved up or down in the folder hierarchy you have to check all the links.

Thanks for your suggestion, I might be able to use it as a temporary fix until I can use absolute pathing again. Even though I don't like the idea of changing my code so that the testing library works, it feels weird.

@warrensplayer warrensplayer added CT Issue related to component testing routed-to-ct labels Oct 25, 2022
@amehta265 amehta265 assigned amehta265 and unassigned warrensplayer Oct 26, 2022
@rockindahizzy
Copy link
Contributor

@lmiller1990
Copy link
Contributor

Debugging this one... looks like we have some special logic for imports in JS here: https://github.com/cypress-io/cypress/blob/develop/npm/webpack-dev-server/src/makeDefaultWebpackConfig.ts#L84 but it's likely not applied for scss imports.

Also a high level google search yields something similar relating to webpack and scss. I'll keep investigating.

@lmiller1990
Copy link
Contributor

Does Angular provide any concrete recommendations for using url? The reason I ask is, looking into sass-loader (which is what Angular uses), they specifically state linked assets must be relative to the output. I am surprised the relative url DOES work - this contradicts what the webpack docs say about sass loader.

@ZachJW34
Copy link
Contributor

You might want to checkout out the --base-href build configuration option. I know it is used for assets and style loading, and we do host the dev-server with __cypress/src base-href.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Dec 12, 2022

Interesting - I found another work around, which is just to re-proxy any non absolute requests (eg on the same domain) here to the dev-server endpoint, but it's a bit of a hack. I will try playing with --base-href and see if this helps.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Dec 12, 2022

I tried various settings for baseHref in angular.json. It is definitely picked up by the config, but does not seem to have any impact on fixing this bug. I suspect baseHref is only used for production builds: https://angular.io/guide/deployment (search base-href).

Edit: yes, baseUrl is only for production builds... I tried building with __cypress/src:

image

@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: in progress labels Dec 13, 2022
@lmiller1990
Copy link
Contributor

lmiller1990 commented Dec 13, 2022

I am going to try something a bit more heavy handed and see if it breaks anything. This might solve some other outstanding issues around incorrect asset requests, too. There's probably a more correct, Angular specific way to solve this (which might require some unexposed internal API?), but doing these case-by-case fixes is starting to get a bit hard to maintain. Maybe this "just proxy it" option might be better in the long run.

Draft PR: #25120

Ah, this breaks cy.intercept for component testing, by the looks of things... I'll continue to look into this.

@lmiller1990
Copy link
Contributor

Update... trying something out: cb2f23b

My idea is forward all traffic to localhost to the dev-server instead (in Component Testing). This should fix a number of issues where the underlying framework configuration is not correctly respected, without us having to fix it on a case-by-case basis. It might let us removed some case-by-case code, like c93b652#diff-7031da07fd51256fb2dc14db39454b4e8ec0f8f90ab283511c2bf847e790eeadR165-R173.

The edge case is if users want to intercept traffic to localhost via cy.intercept. This commit handles that case: cb2f23b - if it's a candidate for intercepting, we do NOT forward it to localhost, and let Cypress handle the intercept.

This might break things - if everything is green, we can probably consider moving ahead with this. If not, I think we will need to explore more framework (eg Angular specific) fixes to solve this issue.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Dec 15, 2022

I think the problem is related to resolving images in SCSS via resolve-url-loader: https://github.com/bholloway/resolve-url-loader/blob/v5/packages/resolve-url-loader/docs/how-it-works.md. I spent quite a bit of time on this but I couldn't figure it out - I tried writing a custom join function, like the docs recommend, but it never gets called.

They've also got a root URL, which looks like what we want, but it's not working for me, either:

"root | string | unset |   | This string, possibly empty, is prepended to absolute URIs. Absolute URIs are only processed if this option is set.".

It always errors:

"root" option must be an empty string or an absolute path to an existing directory

I guess this doesn't work with a virtual filesystem? 🤔

@lmiller1990
Copy link
Contributor

lmiller1990 commented Dec 15, 2022

Ahh I did it!

image

Several hacks, let me write it down before I forget.

  1. I removed the file system check here: https://github.com/bholloway/resolve-url-loader/blob/v5/packages/resolve-url-loader/index.js#L129. Now we can progress.
  2. In node_modules/@angular-devkit/build-angular/src/webpack/configs/styles.js, I used the root option in the styleLanguages array under 'scss' and set it to /__cypress/src (which is devServerPublicPathRoute inside Cypress.
  3. Confusing I still see errors in the console. Ignoring those, I visited the test, and the images are loaded.

Here's a screenshot showing the file. Line 245 is the trick.

image

Obviously hacking into resolve-url-loader is not really an option, but this isolated this issue: Angular's config, specifically the resolve-url-loader, does not know about the Cypress dev server url. If we can fix this somehow, it will work.

What we could do is set sourceRoot, which is just ${projectRoot}/${src}. So + / + <SRC_ROOT>.

@lmiller1990
Copy link
Contributor

I did it, I have fixed the bug. I will make a PR. This was very time consuming and painful, and I am glad it's finally fixed.

@cypress-bot cypress-bot bot added stage: review and removed stage: needs review The PR code is done & tested, needs review labels Dec 16, 2022
@lmiller1990
Copy link
Contributor

lmiller1990 commented Dec 16, 2022

#25191

url in .css is still an issue. For now, just use scss instead of css - it's configured out of the box for Angular. I'm working on a proper fix that will solve this in all webpack-based projects, #25120 was the first attempt but it isn't quite correct.

For now #25191 will be a quick and low risk fix, letting Angular users support url in sass and scss files, correctly routing the assets.

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
6 participants