Fingerprinting of public files does not work in 0.11.1

[tschoartschi]

When upgrading @glimmer/application-pipeline from 0.10.0 to 0.11.1 fingerprinting for files in public does not work anymore. It seems like the public files are not available to broccoli-asset-rev because when you look at tmp/asset_rewrite-input_base_path-cAJ8rDeV.tmp only app.css, app.js, app.js.map and index.html are available.

More details can be found in the following repo: https://github.com/tschoartschi/glimmer-application-pipeline-fingerprinting-bug

[tschoartschi]

@rwjblue did anyone had time to look into this issue? Just to make sure that it's not a problem with my setup. In case you need more information, let me know 😃 thanks

[rondale-sc]

@tschoartschi I wonder if that is related to broccoli 2? I ran into a problem with accessibility when doing something like that recently. Maybe try running EMBER_CLI_BROCCOLI_2=false and see if you see the same thing.

[tschoartschi]

@rondale-sc thanks for the response 😄 I don't think it's because of broccoli 2. I just run the following script on the command line: export EMBER_CLI_BROCCOLI_2=false && rm -rf dist tmp && ember s inside of the github repo I linked above and there is still no fingerprinting for assets in the public folder

[rondale-sc]

Ah, I see. I'm not sure then, how would I reproduce your error to confirm what you've found?

[tschoartschi]

@rondale-sc I have created a repo with a readme which explains what the problem is. Basically I run into the problem that the files which are in the public folder don't get fingerprints after upgrading to 0.11.1

More details can be found in the following repo: https://github.com/tschoartschi/glimmer-application-pipeline-fingerprinting-bug

[rondale-sc]

I bisected and found that this is the culprit:

https://github.com/glimmerjs/glimmer-application-pipeline/commit/d6ebbb5d380561a43f38bfe66c6d0b66adaf9738

I unfortunately don't have time to look into it much further today, but that commit is when public assets stopped being fingerprinted

[tschoartschi]

@rondale-sc thanks for looking into it 👍 maybe I can have a look later as well

[rwjblue]

Yep, good catch @rondale-sc that PR definitely is the cause. This line should be moved:

https://github.com/glimmerjs/glimmer-application-pipeline/blob/c48dcaadf34337e4dbb1ba48f48628032e3d61fc/lib/broccoli/glimmer-app.ts#L148

From the package method down into the end of the toTree method. That should fix the fingerprinting issues.

[rondale-sc]

Ah, I thought it was something about the timing of the addon proccessing based off of what @tschoartschi said. Excellent catch.

[tschoartschi]

@rondale-sc yes this is exactly what I observed. It's not a timing issue (at least not in the sense of a race condition) but it's a problem that postprocessTree is called too early, as @rwjblue already pointed out.

I tried the changes suggested by @rwjblue locally on my machine and it worked. What I did was the following:

move this line: https://github.com/glimmerjs/glimmer-application-pipeline/blob/c48dcaadf34337e4dbb1ba48f48628032e3d61fc/lib/broccoli/glimmer-app.ts#L148 to the toTree method. The toTree method looks now like this:

  public toTree(): Tree {
    if (this.env === "test") {
      return this.testPackage();
    }

    let trees = [
      this.javascriptTree(),
      this.cssTree(),
      this.hbsTree(),
      this.htmlTree()
    ].filter(Boolean) as Tree[];

    let appTree = new MergeTrees(trees);

    let packagedAppTree new MergeTrees([this.publicTree(), this.package(appTree)]);
    return addonProcessTree(this.project, "postprocessTree", "all", packagedAppTree);
  }

If this makes sense I can try to create a pull request. Just let me know if this is the right direction :-)