feat: dynamic mapping expansions + fix CI build failures#2
feat: dynamic mapping expansions + fix CI build failures#2
Conversation
…rrors Co-authored-by: kylebrodeur <811103+kylebrodeur@users.noreply.github.com>
Co-authored-by: kylebrodeur <811103+kylebrodeur@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “dynamic mapping” mode to preserve unmapped fields during map()/reverseMap() operations, and updates project configuration so the package can build/test outside the original monorepo.
Changes:
- Inline TypeScript compiler options in
tsconfig.json(removing missing monorepo base config dependency). - Add
MapOptions+ overloads formap()/reverseMap()to support{ dynamicMapping: true }pass-through behavior. - Expand documentation, changelog entries, and tests for the new option.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Removes monorepo extends and inlines compiler options for standalone builds. |
| src/MappedServiceBase.ts | Implements dynamicMapping option with overloads for typed return widening. |
| src/types.ts | Adds exported MapOptions type describing mapping options. |
| src/index.ts | Exports MapOptions from the package entrypoint and documents the new option. |
| tests/mapping.test.ts | Adds runtime tests and type assertions around dynamic mapping behavior. |
| README.md | Documents dynamic mapping feature and updates API reference accordingly. |
| CHANGELOG.md | Adds an Unreleased entry describing dynamic mapping + build/tooling fixes. |
| package.json | Adds dev tooling dependencies and formats files array. |
| package-lock.json | Locks the new dev dependency tree. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
|
|
||
| it('reverseMap() does not include unmapped fields by default', () => { | ||
| const result = mapper.reverseMap({ isEnterprise: false, commerceType: 'B2C', extra_info: 42 }); |
There was a problem hiding this comment.
This test calls reverseMap() with an object literal containing extra_info but without { dynamicMapping: true }. With the new overloads, the default reverseMap() signature only accepts Partial<MappedType<...>>, so this won’t type-check due to excess property checks. Consider casting the input in the test (or constructing it in a way that avoids excess property checks) to reflect the intended JS/runtime behavior being tested.
| const result = mapper.reverseMap({ isEnterprise: false, commerceType: 'B2C', extra_info: 42 }); | |
| const input = { isEnterprise: false, commerceType: 'B2C', extra_info: 42 }; | |
| const result = mapper.reverseMap(input); |
| if (options?.dynamicMapping) { | ||
| for (const [key, value] of Object.entries(source)) { | ||
| if (!mappedExternalKeys.has(key) && value !== undefined) { | ||
| result[key] = value; | ||
| } |
There was a problem hiding this comment.
When dynamicMapping is enabled, the pass-through loop can overwrite keys already produced by the explicit mapping (e.g., if the source contains an unmapped key that happens to match an internal mapped key). This can silently change mapped values. Consider preventing pass-through from overwriting any key that already exists in result (or define/implement a clear precedence rule, with mapped fields typically winning).
|
|
||
| if (options?.dynamicMapping) { | ||
| for (const [key, value] of Object.entries(target)) { | ||
| if (!mappedInternalKeys.has(key) && value !== undefined) { |
There was a problem hiding this comment.
Same overwrite risk in reverseMap(): with dynamicMapping enabled, pass-through keys from target can collide with already-mapped external keys and overwrite the mapped output. Consider skipping pass-through for any key that is already present in result (or otherwise ensure mapped values can’t be overridden).
| if (!mappedInternalKeys.has(key) && value !== undefined) { | |
| if (!mappedInternalKeys.has(key) && !(key in result) && value !== undefined) { |
| "engines": { | ||
| "node": ">=18.0.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@typescript-eslint/eslint-plugin": "^8.56.1", | ||
| "@typescript-eslint/parser": "^8.56.1", | ||
| "eslint": "^10.0.3", | ||
| "typescript": "^5.9.3", | ||
| "vitest": "^4.0.18" |
There was a problem hiding this comment.
engines.node is set to ">=18.0.0", but the newly added dev tooling (via transitive deps) requires newer Node versions (e.g., ESLint 10 requires Node ^20.19.0 and Vitest 4 requires Node ^20). This will break installs/CI on Node 18. Either bump the package’s Node engine to match (e.g., >=20) or pin eslint/vitest/@typescript-eslint to versions that support Node 18.
Implements proposal #2 (Dynamic Mapping Expansions) and resolves the broken standalone build caused by a missing monorepo-root
tsconfig.base.json.Description
Build fix:
tsconfig.jsonreferenced../../tsconfig.base.jsonwhich doesn't exist outside the original monorepo. Compiler options are now inlined.devDependencies(typescript,vitest,eslint,@typescript-eslint/*) added topackage.jsonso the package is fully self-contained.Dynamic Mapping Expansions:
map()andreverseMap()now accept an optionalMapOptionssecond argument. When{ dynamicMapping: true }is set, fields absent from the mapping are passed through unchanged instead of being silently dropped — useful when only part of the incoming schema is known upfront.TypeScript overloads keep the return type narrow (
MappedType<TSource, TMapping>) for the common case and widen to& Record<string, unknown>only whendynamicMapping: trueis explicit — zero impact on existing call sites.Type of change
Checklist
Additional Notes
MapOptionsis now a public export from the package entrypoint.undefinedfield skipping, default (non-dynamic) behaviour, andexpectTypeOfassertions for both overload branches.[key: string]: unknownindex signature has been added to theApiRowtest interface (required to pass dynamic fields into the mapper).Original prompt
Read the following proposals. Take into account other branches and don't duplicate work. Create a plan for review after seeing if this is good idea.
Here’s a recap of the other enhancement proposals for
@kylebrodeur/type-safe-mapping, along with their goals, benefits, and implementation considerations:1. Runtime Validation for Field Mappings (Already discussed)
2. Dynamic Mapping Expansions
Description:
Example:
Benefits:
Implementation:
dynamicMappingoption tomap()andreverseMap().Breaking Changes:
dynamicMappingflag.3. Support for Key-Value Field Type Validation
Description:
fieldMappingstructure to include field-level options such as the expected data type, default values, or optionality, similar to schema validators.Example:
Benefits:
Implementation:
fieldMappingstructure to accept objects instead of string mappings.MappedTypeutility to infer the target type from this enhanced mapping structure.Breaking Changes:
fieldMappingstructure fundamentally changes.4. TypeSafeMap Utility
Description:
TypeSafeMapclass that implements stricter type guarantees for key-value mappings. This complements the schema-based mapping functionality for developers working with dynamic maps.Example:
Benefits:
Map<K, V>.Maps in dynamic scenarios.Implementation:
Mapto validate types ongetandsetoperations.fieldMappingfor predefining key-value constraints.Breaking Changes:
5. Enhanced Error Messages and Logging
Description:
Benefits:
Example Error Message:
Implementation:
debug: trueoption tomap()for tracing workflows.Breaking Changes:
6. Extend
MappedServiceBasefor Multi-Layer MappingsDescription:
Example:
Benefits:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.