Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

fix(externalRegistry): prevent undefined entries #150

Merged
merged 5 commits into from
Aug 24, 2023

Conversation

JAdshead
Copy link
Contributor

@JAdshead JAdshead commented Aug 23, 2023

Description

Limits the chance of external registries from having undefined entries
Also included JSdoc declarations for external registry

Motivation and Context

fixes bug when integrated with One App.

How Has This Been Tested?

unit tests, tested locally by installing in to one-app server and running one-apps integration test suite. integration tests failed before changes and succeed after.

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (adding or updating documentation)
  • Dependency update

Checklist:

  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • My changes are in sync with the code style of this project.
  • There aren't any other open Pull Requests for the same issue/update.
  • These changes should be applied to a maintenance branch.
  • This change requires cross browser checks.
  • This change impacts caching for client browsers.
  • This change adds additional environment variable requirements for Holocron users.
  • I have added the Apache 2.0 license header to any new files created.

What is the Impact to Developers Using Holocron?

@JAdshead JAdshead requested review from a team August 24, 2023 14:32
* @param {string} externals.[externalName].semanticRange semantic range module will accept
* @param {string} externals.[externalName].integrity hash value of fallback external
*/
function setModulesRequiredExternals({ moduleName, externals = {} }) {
Copy link
Member

Choose a reason for hiding this comment

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

If externals are undefined, should we be setting anything at all?

Copy link
Contributor Author

@JAdshead JAdshead Aug 24, 2023

Choose a reason for hiding this comment

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

i thought the same, landed on if a moduleName is provided it would be expected to be entered into the registry. Im not 100% tied to this.

Copy link
Member

Choose a reason for hiding this comment

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

But if no externals are being passed, then the module technically doesn't require any externals at that point in time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

which is why i think it could be empty.

Co-authored-by: Matthew Mallimo <matthew.c.mallimo@aexp.com>
@JAdshead JAdshead requested a review from a team August 24, 2023 20:08
@JAdshead JAdshead merged commit 0676c06 into main Aug 24, 2023
@JAdshead JAdshead deleted the fix/invalid-registry-entry branch August 24, 2023 21:10
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants