Only populate the minimum needed assets when doing inline replacement

[Munter]

When the assets written back to disk are the original ones there is really no reason to spend time on populating any parts of the dependency graph that don't affect the outcome.

Relations that should be populated include:

• HtmlStyle
• HtmlAnchor
• HtmlIframe
• HtmlTemplate,
• HtmlConditionalComment
• HtmlFrame
• HtmlIframe
• HtmlIframeSrcDoc
• HtmlImport
• HtmlInlineScriptTemplate
• HtmlNoscript
• HtmlScript ? Not sure we can cover JS paths
• HtmlStyleAttribute
• HtmlSvgIsland
• CssImport
• CssFontFaceSrc

Did I forget any? Or is this even a sane approach?

+@papandreou

[Munter]

Given the new v4 API, could such a query be simplified to something like this?

{
  followRelations: {
    crossorigin: false,
    to: {
      type: ['Html', 'Css', 'Font'] // Not sure the font type exists yet
    }
  }
}

[papandreou]

HtmlScript ? Not sure we can cover JS paths

We could theoretically pick up references to stylesheets and templates referenced from JavaScript if they're using the .toString('url') syntax. It's presently not used in subsetFonts, but we could just add it. If we do it, we should remember to add a { noscript: false } predicate to it, or maybe invert the predicate so it's { script: true } and the stylesheets in <noscript> get tagged with { script: false } :)

[papandreou]

The list looks complete otherwise. Note that HtmlIFrame and HtmlIFrameSrcDoc are spelled with a capital I.

If we add the required features for HtmlScript, we should also add HtmlInlineEventHandler, JavaScriptStaticUrl, JavaScriptImportScripts, JavaScriptServiceWorkerRegistration, and JavaScriptWebWorker to the list. And JavaScriptImport and JavaScriptExport on master. And HtmlDataBind and HtmlKnockoutContainerless if we fancy supporting those Knockout.js specific JavaScript fragments.

Regarding HtmlSvgIsland, that one's fun. Yeah, with a little bit of luck we'd be able to extend the tracing to include text inside inline <svg> like the hacks for HtmlNoscript and HtmlConditionalComment. I guess we'll have to do some work to make selectors in the containing document match the SVG nodes. On the other hand, the nodes also exist in the containing document, so maybe we don't even have to do anything for that to work, but we could still dive into the SVG to pick up stylesheet references. Would have to experiment a bit with that :)

Oh, FileRedirect and HttpRedirect should also be added. Otherwise we won't follow eg. an HTTP redirect to a stylesheet, or a "virtual file redirect" from eg. foo/ to foo/index.html.

[Munter]

It's going to be hard to reason about source order of JS included CSS, and thus also specificity. That will most likely be a large source of errors

[papandreou]

You're right about that. Could argue that we need to dive into executing the code and doing a new trace whenever the DOM mutates to truly cover that case.

[papandreou]

Given the new v4 API, could such a query be simplified to something like this?

{
  followRelations: {
    crossorigin: false,
    to: {
      type: ['Html', 'Css', 'Font'] // Not sure the font type exists yet
    }
  }
}

That won't work right now because Font is just a superclass for Woff, Woff2, Ttf, Eot, and Otf -- so those assets don't have a type of Font -- but they have isFont: true.

If we implement the idea we discussed yesterday, the query could look something like:

{
  followRelations: {
    crossorigin: false,
    to: [ { Html: true }, { Css: true }, { Font: true } ]
  }
}

... which isn't really great either. Maybe we could have a shorthand so that a string matched against an asset means that the asset needs to have a truthy property with that name so it could be:

{
  followRelations: {
    crossorigin: false,
    to: [ 'Html', 'Css', 'Font' ]
  }
}

[Munter]

I'm fine with using Woff, Woff2, Ttf, Eot, and Otf. The point was more to be able to express the population query in the shortest and most understandable way possible. In this case there are so many types of relations that can lead to what I want in the graph, but only a quite limited set of asset types

[papandreou]

Yeah, I guess this is a good example of why we want to retain the ability to query by asset types. I'm happy assetgraph/assetgraph#793 went in so we don't lose the ability to do that for assets that haven't loaded yet.

[papandreou]

For the record, the new query syntax would be:

{
  followRelations: {
    crossorigin: false,
    to: {
      type: {
        $in: [ 'Html', 'Css', 'Font' ]
      }
    }
  }