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

Only add to manifest when necessary #123

Merged

Conversation

brendenpalmer
Copy link
Contributor

@brendenpalmer brendenpalmer commented Feb 2, 2022

This PR updates the asset manifest logic to only add a bundle to the manifest if that bundle actually has assets.

yarn test:

yarn test
yarn run v1.22.10
$ ember test
WARNING: Node v14.15.4 is not tested against Ember CLI on your platform. We recommend that you use the most-recent "Active LTS" version of Node.js. See https://git.io/v7S5n for details.
DEPRECATION: The integration of jQuery into Ember has been deprecated and will be removed with Ember 4.0. You can either opt-out of using jQuery, or install the `@ember/jquery` addon to provide the jQuery integration. Please consult the deprecation guide for further details: https://emberjs.com/deprecations/v3.x#toc_jquery-apis
⠋ BuildingBrowserslist: caniuse-lite is outdated. Please run:
npx browserslist@latest --update-db
Environment: test
cleaning up...
Built project successfully. Stored in ...
ok 1 Chrome 98.0 - [2 ms] - ESLint | addon-test-support: addon-test-support/loaded-asset-state.js
ok 2 Chrome 98.0 - [0 ms] - ESLint | addon-test-support: addon-test-support/preload-assets.js
ok 3 Chrome 98.0 - [1 ms] - ESLint | addon: addon/errors/asset-load.js
ok 4 Chrome 98.0 - [0 ms] - ESLint | addon: addon/errors/bundle-load.js
ok 5 Chrome 98.0 - [0 ms] - ESLint | addon: addon/errors/load.js
ok 6 Chrome 98.0 - [0 ms] - ESLint | addon: addon/loaders/css.js
ok 7 Chrome 98.0 - [0 ms] - ESLint | addon: addon/loaders/js.js
ok 8 Chrome 98.0 - [0 ms] - ESLint | addon: addon/loaders/utilities.js
ok 9 Chrome 98.0 - [1 ms] - ESLint | addon: addon/services/asset-loader.js
ok 10 Chrome 98.0 - [0 ms] - ESLint | app: app/config/asset-manifest.js
ok 11 Chrome 98.0 - [0 ms] - ESLint | app: app/instance-initializers/load-asset-manifest.js
ok 12 Chrome 98.0 - [0 ms] - ESLint | app: app/services/asset-loader.js
ok 13 Chrome 98.0 - [145 ms] - Acceptance | asset-load: visiting a route which loads a bundle
ok 14 Chrome 98.0 - [77 ms] - Acceptance | asset-load: visiting a route which fails to load a script removes the node from DOM
ok 15 Chrome 98.0 - [0 ms] - ESLint | app: app.js
ok 16 Chrome 98.0 - [1 ms] - ESLint | app: initializers/router-ext.js
ok 17 Chrome 98.0 - [0 ms] - ESLint | app: resolver.js
ok 18 Chrome 98.0 - [0 ms] - ESLint | app: router.js
ok 19 Chrome 98.0 - [0 ms] - ESLint | app: routes/asset-error.js
ok 20 Chrome 98.0 - [0 ms] - ESLint | tests: acceptance/asset-load-test.js
ok 21 Chrome 98.0 - [0 ms] - ESLint | tests: helpers/destroy-app.js
ok 22 Chrome 98.0 - [0 ms] - ESLint | tests: helpers/module-for-acceptance.js
ok 23 Chrome 98.0 - [0 ms] - ESLint | tests: helpers/resolver.js
ok 24 Chrome 98.0 - [0 ms] - ESLint | tests: helpers/start-app.js
ok 25 Chrome 98.0 - [1 ms] - ESLint | tests: test-helper.js
ok 26 Chrome 98.0 - [0 ms] - ESLint | tests: unit/asset-manifest-test.js
ok 27 Chrome 98.0 - [0 ms] - ESLint | tests: unit/errors/asset-load-test.js
ok 28 Chrome 98.0 - [0 ms] - ESLint | tests: unit/errors/bundle-load-test.js
ok 29 Chrome 98.0 - [0 ms] - ESLint | tests: unit/errors/load-test.js
ok 30 Chrome 98.0 - [0 ms] - ESLint | tests: unit/services/asset-loader-test.js
ok 31 Chrome 98.0 - [1 ms] - ESLint | tests: unit/test-helpers/loaded-asset-state-test.js
ok 32 Chrome 98.0 - [1 ms] - Unit | asset-manifest: node-asset-manifest is generated properly
ok 33 Chrome 98.0 - [0 ms] - Unit | asset-manifest: loads the node-asset-manifest if present
ok 34 Chrome 98.0 - [1 ms] - Unit | asset-manifest: loads the manifest from the meta tag if available
ok 35 Chrome 98.0 - [0 ms] - Unit | asset-manifest: throws an error if unable to load the manifest
ok 36 Chrome 98.0 - [1 ms] - Unit | Error | asset-load: constructor() - accepts an asset and the original error
ok 37 Chrome 98.0 - [0 ms] - Unit | Error | asset-load: toString() - has correct name and message
ok 38 Chrome 98.0 - [0 ms] - Unit | Error | asset-load: retryLoad() - calls loadAsset on the loader
ok 39 Chrome 98.0 - [0 ms] - Unit | Error | bundle-load: constructor() - accepts a bundleName and errors array
ok 40 Chrome 98.0 - [0 ms] - Unit | Error | bundle-load: toString() - has correct name and message
ok 41 Chrome 98.0 - [0 ms] - Unit | Error | bundle-load: retryLoad() - calls loadBundle on the loader
ok 42 Chrome 98.0 - [0 ms] - Unit | Error | load: constructor() - accepts a message and a loader
ok 43 Chrome 98.0 - [0 ms] - Unit | Error | load: toString() - has correct name and message
ok 44 Chrome 98.0 - [0 ms] - Unit | Error | load: retryLoad() - throws an error
ok 45 Chrome 98.0 - [35 ms] - Unit | Service | asset-loader: loadBundle() - throws an error if no asset manifest has been provided
ok 46 Chrome 98.0 - [29 ms] - Unit | Service | asset-loader: loadBundle() - throws an error if asset manifest does not list any bundles
ok 47 Chrome 98.0 - [31 ms] - Unit | Service | asset-loader: loadBundle() - throws an error if asset manifest does not contain the bundle
ok 48 Chrome 98.0 - [35 ms] - Unit | Service | asset-loader: loadBundle() - returns a promise that resolves with the name of the loaded bundle
ok 49 Chrome 98.0 - [37 ms] - Unit | Service | asset-loader: loadBundle() - subsequent calls return the same promise
ok 50 Chrome 98.0 - [35 ms] - Unit | Service | asset-loader: loadBundle() - rejects with a BundleLoadError
ok 51 Chrome 98.0 - [35 ms] - Unit | Service | asset-loader: loadBundle() - a rejection allows retrying the load
ok 52 Chrome 98.0 - [35 ms] - Unit | Service | asset-loader: loadBundle() - subsequent call after rejection returns a new promise
ok 53 Chrome 98.0 - [35 ms] - Unit | Service | asset-loader: loadBundle() - retrying a load twice returns the same promise
ok 54 Chrome 98.0 - [30 ms] - Unit | Service | asset-loader: loadAsset() - throws an error if there is no loader defined for the asset type
ok 55 Chrome 98.0 - [34 ms] - Unit | Service | asset-loader: loadAsset() - returns a promise that resolves with the loaded asset information
ok 56 Chrome 98.0 - [33 ms] - Unit | Service | asset-loader: loadAsset() - subsequent calls return the same promise
ok 57 Chrome 98.0 - [33 ms] - Unit | Service | asset-loader: loadAsset() - rejects with an AssetLoadPromise
ok 58 Chrome 98.0 - [35 ms] - Unit | Service | asset-loader: loadAsset() - a rejection allows retrying the load
ok 59 Chrome 98.0 - [34 ms] - Unit | Service | asset-loader: loadAsset() - subsequent call after rejection returns a new promise
ok 60 Chrome 98.0 - [33 ms] - Unit | Service | asset-loader: loadAsset() - retrying a load twice returns the same promise
ok 61 Chrome 98.0 - [24 ms] - Unit | Service | asset-loader: loadAsset() - js - handles successful load
ok 62 Chrome 98.0 - [24 ms] - Unit | Service | asset-loader: loadAsset() - js - handles failed load
ok 63 Chrome 98.0 - [36 ms] - Unit | Service | asset-loader: loadAsset() - js - does not insert additional script tag if asset is in DOM already
ok 64 Chrome 98.0 - [33 ms] - Unit | Service | asset-loader: loadAsset() - js - sets async false to try to guarantee execution order
ok 65 Chrome 98.0 - [38 ms] - Unit | Service | asset-loader: loadAsset() - css - handles successful load
ok 66 Chrome 98.0 - [26 ms] - Unit | Service | asset-loader: loadAsset() - css - handles failed load
ok 67 Chrome 98.0 - [34 ms] - Unit | Service | asset-loader: loadAsset() - css - does not insert additional link tag if asset is in DOM already
ok 68 Chrome 98.0 - [34 ms] - Unit | Service | asset-loader: defineLoader() - overwrites existing asset loader types
ok 69 Chrome 98.0 - [29 ms] - Unit | Service | asset-loader: pushManifest() - throws an error when merging two manifests with the same bundle
ok 70 Chrome 98.0 - [26 ms] - Unit | Test Helper | loaded-asset-state: resetLoadedAssetState removes new script tags, new link tags, and new modules
ok 71 Chrome 98.0 - [0 ms] - ember-qunit: Ember.onerror validation: Ember.onerror is functioning properly

yarn test:node:

yarn test:node
yarn run v1.22.10
$ mocha node-tests


  asset-manifest-generator
    build
      ✓ generates an asset manifest from an input tree (401ms)
      ✓ handles custom supportedTypes
      ✓ prepends custom paths to the generated uris
      ✓ can ignore specific files
      ✓ can ignore specific files using regex
      ✓ merges with an existing manifest
      ✓ throws an error when a bundle collision occurs

  bundleInfoFromAssetEntry
    ✓ converts basic paths from walkSync into bundle entry objects
    ✓ converts scoped paths from walkSync into bundle entry objects

  asset-manifest-inserter
    build
      ✓ only modifies index.html
      ✓ uses the correct file
      ✓ successfully modifies the manifest
      ✓ uses a custom transformer to modify the manifest

  generate-asset-manifest
    ✓ adds an asset manifest to the supplied tree (54ms)
    ✓ adds an asset manifest with custom supportedTypes (46ms)
    ✓ uses a custom bundlesLocation and properly prepends it to generated URIs (40ms)
    ✓ can ignore specific files (46ms)
    ✓ merges an existing manifest into the new one (45ms)

  manifest-generator
    contentFor
      ✓ returns a meta tag with a placeholder for head-footer (1863ms)
      ✓ returns nothing when using the noManifest option (1041ms)
      ✓ returns nothing when type is not head-footer (1076ms)
    postprocessTree
      ✓ returns the input tree if not of type all (1584ms)
      ✓ returns the input tree when using the noManifest option (1055ms)
      ✓ generates an asset manifest, merges it into the given tree, and inserts it into index.html (1125ms)
      ✓ inserts the asset manifest into a custom index path (1126ms)
      ✓ uses the generateURI method to create the URIs (946ms)
      ✓ uses filesToIgnore (574ms)

  meta-handler
    replacer
      ✓ does not throw on empty input
      ✓ supports ">" in bundle
      ✓ only replaces the first occurrence
      ✓ supports newlines


  31 passing (11s)

@rwjblue rwjblue added the bug label Feb 2, 2022
@rwjblue
Copy link
Member

rwjblue commented Feb 2, 2022

Unfortunately, our CI is totally hosed and we need to do a bit of repo maintenance here. Mind sharing the results of yarn test && yarn test:node (just to confirm things are still passing)?

@brendenpalmer
Copy link
Contributor Author

@rwjblue yeah i ran locally, i'll update the description

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

Successfully merging this pull request may close these issues.

None yet

2 participants