Skip to content

chore: Improve handling of custom ORD content#443

Merged
mlakov merged 16 commits into
mainfrom
chore/improve-handling-of-custom-ord-content
May 26, 2026
Merged

chore: Improve handling of custom ORD content#443
mlakov merged 16 commits into
mainfrom
chore/improve-handling-of-custom-ord-content

Conversation

@mlakov

@mlakov mlakov commented May 14, 2026

Copy link
Copy Markdown
Contributor

Refactor Custom ORD Content Merging with Strategy-Based Approach

Refactor

♻️ Replaced the generic object traversal merge logic for custom ORD content with an explicit, strategy-based approach using a MERGE_STRATEGIES map. This improves correctness, predictability, and eliminates the need for workarounds like cleanNullProperties.

Changes

  • lib/extend-ord-with-custom.js: Major refactor of the merging logic:

    • Removed cleanNullProperties and the old patchGeneratedOrdResources functions.
    • Introduced a MERGE_STRATEGIES map defining explicit per-property merge behavior (scalar replace, object assign, or smart array merge by key).
    • Added prune helper to remove null/undefined values from merged entities.
    • Added merge helper for smart array-of-objects merging using configurable identity keys (e.g., ordId, groupId).
    • getCustomORDContent now consistently returns {} instead of undefined when no custom file is configured or the file is missing.
    • compareAndHandleCustomORDContentWithExistingContent now uses the strategy map to handle properties exclusive to either side or present in both.
    • Removed the warning for top-level primitive properties — they are now handled via the strategy map.
    • Removed the CONTENT_MERGE_KEY dependency from constants.
  • __tests__/unit/extend-ord-with-custom.test.js: Updated test descriptions and expectations to reflect {} return values instead of undefined; removed the deprecated "warn on top-level primitive" test case.

  • __tests__/bookshop/ord/custom.ord.json: Removed conflicting top-level properties (namespace, openResourceDiscovery) that were only needed for the now-removed warning test.

  • __tests__/unit/__snapshots__/extend-ord-with-custom.test.js.snap: Updated snapshots to reflect the removal of the deprecated test case and corrected localId values.

  • __tests__/unit/__snapshots__/ord.e2e.test.js.snap: Updated snapshots to reflect corrected ordering and content of merged packages, including fixed shortDescription and version values from the improved merge strategy.

  • 🔄 Regenerate and Update Summary
PR Bot Information

Version: 1.20.51

  • Event Trigger: issue_comment.edited
  • Correlation ID: 6c3ac90f-9ddf-4fa7-ae63-ac5685b83b3a

@mlakov
mlakov requested review from swennemers and zongqichen May 14, 2026 12:10
hyperspace-pr-bot[bot]

This comment was marked as outdated.

@mlakov mlakov added ready for review run-e2e Trigger e2e test pipeline labels May 20, 2026
@mlakov
mlakov marked this pull request as ready for review May 20, 2026 11:23
@mlakov

mlakov commented May 20, 2026

Copy link
Copy Markdown
Contributor Author

@zongqichen, @swennemers - can you, please, review ?

hyperspace-pr-bot[bot]

This comment was marked as outdated.

Comment thread lib/extend-ord-with-custom.js
Comment thread lib/extend-ord-with-custom.js
Comment thread lib/extend-ord-with-custom.js Outdated

@zongqichen zongqichen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@mlakov
mlakov merged commit 3786591 into main May 26, 2026
5 checks passed
@mlakov
mlakov deleted the chore/improve-handling-of-custom-ord-content branch May 26, 2026 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review run-e2e Trigger e2e test pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants