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 transform to load and import external CDN assets into the assetgraph root #229

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Munter
Copy link
Member

@Munter Munter commented Dec 6, 2014

Refs assetgraph/assetgraph-builder#148

@papandreou This is very basic in that it only imports CSS and JavaScript. Might want to add web components as well. The test are running a bit slow, do you have any tricks to stub out the CDN assets?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling 3a77078 on uncdnify into 5762c0e on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) when pulling 7d25706 on uncdnify into 5762c0e on master.

@papandreou
Copy link
Member

We can stub the CDN assets out with unexpected-mitm, see the http resolver tests.

Parts of this could also be relevant for SourceMappingURL/SourceURL pointing to an external asset, and for the SRI support where we need to compute hashes of the external assets. We'll need to refine a lot of the queries in buildProduction to do the right thing.

Copy link
Member

@papandreou papandreou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, what about fonts and images etc. refererenced by the adopted assets? We want to pull those in as well, right?

@Munter
Copy link
Member Author

Munter commented Oct 9, 2016

Good point. I'll look into that.

How the heck does this PR drop code coverage by 5%?

// 'HtmlResourceHint',
// 'HtmlScript',
'HtmlSearchLink',
// 'HtmlServiceWorkerRegistration',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be blacklisted as well.

// 'HtmlFluidIconLink',
'HtmlFrame',
'HtmlIFrame',
'HtmlIFrameSrcDoc',
Copy link
Member

@papandreou papandreou Oct 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessary to list as it's a "born" inline relation.

@Munter
Copy link
Member Author

Munter commented Oct 9, 2016

Updated according to feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants