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(build): use html-loader to figureprint images (#3415) #8190

Closed

Conversation

asnowwolf
Copy link

@asnowwolf asnowwolf commented Oct 26, 2017

A quick and dirty PR about using html-loader's "root-relative" urls to figureprint images in assets

demo: https://github.com/asnowwolf/test-ng-cli
see https://github.com/webpack-contrib/html-loader#root-relative-urls

Fix #3415

@asnowwolf
Copy link
Author

@filipesilva

I found that the "root-relative URLs" feature of html-loader may help to solve this problem easily. Could you please review it?

If this resolution is feasible, then I will add the test and don't copy referenced images in assets folder.

@filipesilva
Copy link
Contributor

filipesilva commented Oct 26, 2017

@asnowwolf now that you mention the root relative urls, I remember I had tried to make them work and ran into some trouble... We have better logic to determine what the root is now so maybe that's a moot point.

We have to come up with a battery of tests similar to https://github.com/angular/angular-cli/blob/master/tests/e2e/tests/build/css-urls.ts for html urls, including interpolation, to make sure they work properly.

It might be a bit before I can really come back to this though, because we're doing a couple of big releases now and then have Angular Connect. But I'll put it on my shortlist to investigate again. I know this behavior is very confusing.

@clydin
Copy link
Member

clydin commented Oct 27, 2017

The defining characteristic of files defined as assets via .angular-cli.json is that they are copied verbatim to the output. They shouldn't be modified, fingerprinted, filtered etc. With the setup demonstrated here, the referenced asset will appear in the output twice. Once in the output assets folder and again in the root folder. The second having been processed by Webpack and the intention of the asset configuration is that the files are not processed by Webpack.

The root relative URL handling demonstrated here also differs greatly from the stylesheet handling. The behavior here should ideally match that of the stylesheet processing.

Just using html-loader also does not address the more systemic issue discussed in the various issues. The html-loader assumes the HTML is static (i.e., the source HTML is equivalent to the rendered HTML). A component template (which happens to be HTML compatible) is the source in this case and is taken by Angular and converted at runtime into HTML that will then be rendered. There may be radical differences between the two; including external file references.

@asnowwolf
Copy link
Author

@filipesilva @clydin Thank you for your explanations. I am now more aware of this issue, I will continue to study it. Look forward to your follow-up work.

@asnowwolf asnowwolf closed this Oct 27, 2017
@asnowwolf
Copy link
Author

@clydin I agree assets should not be modified/figureprinted/filtered. I meant maybe we can put them in an absolute path, e.g. /app/images.

@filipesilva
Copy link
Contributor

Hey do you mind if I keep the PR open? It has important insight into the problem.

@filipesilva filipesilva reopened this Nov 2, 2017
@asnowwolf
Copy link
Author

@filipesilva Yep, It's my honor to track this issue through this PR. Look forward to your follow-up.

@clydin
Copy link
Member

clydin commented May 9, 2018

Closing as build functionality has been moving to the angular/devkit repository.

@clydin clydin closed this May 9, 2018
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Images in templates are not fingerprinted
4 participants