Repository navigation
fix(json-api-nestjs-sdk): take the meta symbols from the global registry and export the readers for Angular - #129
Merged
Merged
Conversation
…try and export the readers for Angular Three things review caught in 10.1.0. The symbols were module-local. The package ships two entry points and nothing promises a bundler gives them the same copy of resource-meta: cjs and mjs can resolve apart, or one can be inlined while the other stays external. Two copies would write under one symbol and read under another, and getResourceMeta would answer undefined with no error to show for it. Symbol.for keys them in the global registry instead, scoped by package name so they cannot collide. Both entry points lead to one file today, so this never showed -- which is also why it would have been hard to find later. getResourceMeta, getDocumentMeta and ResourceMeta reached only the root entry point, while an Angular consumer takes its services from ./ngModule and would have needed a second import path for them. They are exported there too. The casts reading dataItem.meta and body.meta predate the field being added to ResourceData in the same change, and were never cleaned up. The build confirms they were redundant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
View your CI Pipeline Execution ↗ for commit c1064ac
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗ ☁️ Nx Cloud last updated this comment at |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three things review caught in
10.1.0.The symbols were module-local
The package ships two entry points and nothing promises a bundler gives them the
same copy of
resource-meta:cjsandmjscan resolve apart, or one can beinlined while the other stays external. Two copies would write under one
symbol and read under another, and
getResourceMetawould answerundefinedwith no error to show for it.
Symbol.forkeys them in the global registry instead, scoped by package name:Both entry points lead to one file today, so this never showed — which is also
why it would have been hard to find later. The test reads the property through
Symbol.for(...)directly rather than through the exported reader, so it pinsthe key being global rather than merely that the reader works.
The readers were missing from
./ngModulegetResourceMeta,getDocumentMetaandResourceMetareached only the rootentry point, while an Angular consumer takes
JsonApiUtilsServiceandJsonApiSdkServicefrom./ngModuleand would have needed a second import pathjust for these. Verified present in the built
.jsand.d.ts.Two leftover casts
(dataItem as { meta?: unknown }).metaand the same forbody.metapredatemetabeing added toResourceDatain the very same change, and were nevercleaned up. The build confirms they were redundant.
Verification
Unit across 9 projects, ts-test, e2e 137/137 on both MikroORM and TypeORM.
json-api-front:build:productionfails on a CSS budget, as on master.🤖 Generated with Claude Code