Skip to content

Conversation

@daniyelnnr
Copy link
Contributor

@daniyelnnr daniyelnnr commented Oct 9, 2025

What is the purpose of this pull request?

This PR restores backward compatibility for individual header constant exports (e.g., TENANT_HEADER, BINDING_HEADER, LOCALE_HEADER, SEGMENT_HEADER) that were removed in #579, causing breaking changes for apps that depend on @vtex/api. It also enhances test coverage for the introduced changes in #579.

  • Restored individual header constant exports in src/constants.ts
  • Each constant references its HeaderKeys counterpart (single source of truth)
  • Added TSDoc @deprecated notices to guide developers toward the modern HeaderKeys API
  • Added comprehensive test coverage verifying both APIs work correctly
  • Maintained dual API support: legacy constants (backward compatible) + modern HeaderKeys object

What problem is this solving?

PR #579 (commit b1952ce) migrated from individual header constant exports to a HeaderKeys object for standardization. This introduced a breaking change because:

  1. Some apps import individual constants from @vtex/api
  2. These imports now resolve to undefined instead of header string values
  3. Apps use undefined as header keys, silently failing to set/read critical headers
  4. Downstream apps receive requests with missing headers (e.g., x-vtex-tenant)
  5. ctx.vtex.tenant becomes undefined, causing runtime errors when destructured without guards

This led to errors like:

  • TypeError: Cannot read property 'locale' of undefined when destructuring tenant
  • Translation helpers fail when both ctx.vtex.binding?.locale and ctx.vtex.tenant?.locale are missing

In order to fix this, we re-export individual constants alongside the new HeaderKeys object, maintaining both APIs.

The test suite now includes additional tests that validate real-world usage patterns observed in VTEX IO apps:

  • Object Property Access Test (src/constants.test.ts:539-562): tests the pattern where constants are used as computed property keys in header objects
    • Validates that no undefined keys are created, which was the root cause of the bug
  • Destructuring Import Test (src/constants.test.ts:564-587): validates CommonJS if destructuring work correctly
    • Ensures constants are not undefined after destructuring (the exact bug we're fixing)
  • VaryHeaders Type Compatibility Test (src/constants.test.ts:589-615): verifies that individual constants work with the VaryHeaders type that changed in Add diagnostics-semconv to standardize some attributes and headers #579
    • Tests type-level compatibility: individual constants should be assignable where VaryHeaders is expected
  • Integration Scenarios Test (src/constants.test.ts:617-669): comprehensive test covering patterns observed in internal GraphQL apps. This ensures to test the full workflow: set headers → send request → read headers.
    • Building request headers objects with format functions (common in GraphQL context links)
    • Conditional header setting (common in server-side rendering middleware)
    • Reading headers from incoming requests (common in context preparation)
    • Based on documented patterns in:
      • src/service/worker/runtime/utils/context.ts (context preparation)
      • src/HttpClient/HttpClient.ts (HTTP client header building)
      • Common VTEX IO app patterns from service-example repository

How should this be manually tested?

Build and test the package:

cd node-vtex-api
yarn install
yarn build
yarn test src/constants.test.ts

Screenshots or example usage

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires change to documentation, which has been updated accordingly.

- Re-export legacy `*_HEADER` named exports (e.g., TENANT_HEADER, REQUEST_ID_HEADER, etc.) as deprecated aliases of `HeaderKeys.*`
- Preserves public API without a major bump while guiding migration via @deprecated JSDoc
- Assert deprecated `*_HEADER` exports exist and mirror `HeaderKeys.*`
- Add checks for critical VTEX headers and expected string values
@daniyelnnr daniyelnnr self-assigned this Oct 9, 2025
@daniyelnnr daniyelnnr added the bug label Oct 13, 2025
- Add tests to verify constants can be used as object keys without runtime errors
- Ensure destructuring from module exports works correctly
- Validate compatibility of constants with VaryHeaders type
- Simulate realistic scenarios for header key usage in IO apps
@daniyelnnr daniyelnnr marked this pull request as ready for review October 13, 2025 19:18
Copy link
Contributor

@silvadenisaraujo silvadenisaraujo left a comment

Choose a reason for hiding this comment

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

LGTM, it was an interesting learning for following header updates.
Could you please update the github actions to run the tests even if the lint fails?

- Update the type of incomingHeaders to Record<string, string> for better type safety
@daniyelnnr
Copy link
Contributor Author

LGTM, it was an interesting learning for following header updates. Could you please update the github actions to run the tests even if the lint fails?

Ok @silvadenisaraujo ! Addressed this in dfd3369.
However some tests are failing prior these changes. I'm already working on fixing it (ref) and until this is finished we can check that the tests for this change are passing.

@daniyelnnr daniyelnnr merged commit 0a92667 into master Oct 15, 2025
0 of 2 checks passed
@daniyelnnr daniyelnnr deleted the fix/semconv-headers branch October 15, 2025 13:39
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.

3 participants