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

au build bundle output and revision hashes are non-deterministic #378

Closed
mattbrooks2010 opened this issue Oct 17, 2016 · 10 comments
Closed
Labels

Comments

@mattbrooks2010
Copy link

I'm submitting a bug report

  • Library Version:
    0.21.0

Please tell us about your environment:

  • Operating System:
    Windows 10

  • Node Version:
    v6.5.0

  • NPM Version:
    3.10.8

  • Language:
    TypeScript 2.0.3

Current behavior:

  1. Run au new selecting TypeScript as the transpiler.
  2. Set build.options.rev to true in aurelia_project/aurelia.json.
  3. Run au build two or more times, without modifying source files or dependencies.

The following example extracts can be seen from the output of each build:

Writing app-bundle-cc53997feb.js...
Writing vendor-bundle-aedd4b31b0.js...

Writing app-bundle-cc53997feb.js...
Writing vendor-bundle-98aa6f3539.js...

The build generates different vendor-bundle-[hash].js files each time. Both the file contents are different (in terms of order of contents) and therefore the revision hashes are different too. Below is a visual diff to illustrate the result:

image

This issue does not seem to affect app bundles. (Although as a separate issue the app bundle filename is referenced from the vendor bundle. This means, when revision hashes are used, a change to the source files will always generate a new vendor bundle revision hash. In my opinion this is a design issue.)

Expected/desired behavior:

  • What is the expected behavior?
    I would expect the output of the bundle files to be deterministic in all cases. The order of the contents should not differ between builds and therefore nor should the revision hashes.
  • What is the motivation / use case for changing the behavior?
    This is important for continuous deployment scenarios. You wouldn't want to deploy a new cache busting bundle filename if the source files and dependencies hadn't changed.

For example, consider deploying a large Aurelia app built using the Aurelia CLI to production multiple times per day. Dependencies have not changed between builds and yet users of the app will be forced to download the bundle script file after each deployment.

@AStoker
Copy link
Contributor

AStoker commented Oct 18, 2016

I noticed this as well. The problem is that when the vendor bundle is created, it doesn't seem to concatenate the modules in the same order consistently. Not sure where/why that is happening though.

@ghidello
Copy link
Contributor

I'm looking to this snippet (I just reformatted a bit the map method)

static create(bundler, config) {
    let bundle = new exports.Bundle(bundler, config);
    let dependencies = config.dependencies || [];

    return Promise.all(
        dependencies
            .filter(x => bundler.itemIncludedInBuild(x))
            .map(dependency => bundler
                .configureDependency(dependency)
                .then(description => bundle.addDependency(description))
            )
    ).then(() => bundle);
}

My guess is that the issue here is related to the fact that the addDependency method (which adds the dependencies to the 'includes' array used by the bundler) is executed as soon as the configureDependency method resolves and so the final dependency order is not guaranteed.
Does it make sense?

@AStoker
Copy link
Contributor

AStoker commented Nov 21, 2016

If that's what's happening, that does make sense. Think you could make a PR for a fix?
Thanks for digging into why this could be happening by the way!

@paulharker
Copy link

paulharker commented Nov 23, 2016

This has a definite effect for us: we are loading jquery and ms-signalr-client (a library that extends jQuery). ms-signalr-client requires that jquery be loaded first, and it is completely arbitrary whether or not it will work based on this bug.

I would hope that creating an entry in aurelia.json like this would result in jquery being loaded first:

  "dependencies": [
          {
            "name": "jquery",
            "path": "../node_modules/jquery/dist",
            "main": "jquery"
          },
          {
            "name": "ms-signalr-client",
            "path": "../node_modules/ms-signalr-client",
            "main": "jquery.signalR.js",
            "deps": [
              "jquery"
            ],
            "exports": "$"
          },

but it does not. I have a workaround of putting the jquery loader in the prepend:

        "prepend": [
          "node_modules/bluebird/js/browser/bluebird.core.js",
          "node_modules/whatwg-fetch/fetch.js",
          "node_modules/jquery/dist/jquery.js",
          "scripts/require.js"
        ],

I am following this issue and hope by the time we go to production in a few months that there might be some movement. Would love to help, but this PR may be above my paygrade as of yet. -- Paul

@EisenbergEffect
Copy link
Contributor

@AStoker Any chance you can look into this?

@AStoker
Copy link
Contributor

AStoker commented Nov 28, 2016

@EisenbergEffect, I can add it to my list, but right now I'm pretty loaded up at work, and we're house hunting too, so it might be a bit before I get to it unfortunately. I was really hoping somebody would submit a PR for this one :)

@alpox
Copy link
Contributor

alpox commented Dec 26, 2016

@AStoker @EisenbergEffect This should be fixed with the integration of #427

@AStoker
Copy link
Contributor

AStoker commented Mar 29, 2017

Issue was either not fully resolved or has returned: #472 (comment)

I just reproduced the issue with a fresh 0.27.0 install creating new default project, i.e.
...
After setting the rev option to true in aurelia.json, i.e.
...
I executed 4 times the command au build and I found 4 different vendor bundles in the script folder. Comparing the files the differences appear to be related to the order of the aurelia dependencies included in the bundle.

@AStoker AStoker reopened this Mar 29, 2017
@JeroenVinke
Copy link
Collaborator

@AStoker I already have a few tests for the Bundler class locally. I'll see if I can add a test case for the order of the dependencies

@AStoker
Copy link
Contributor

AStoker commented Mar 29, 2017

That would be awesome, thanks!

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

No branches or pull requests

7 participants