Skip to content

fix(Camera): allow partial padding on the root padding prop - #4279

Open
giaBaoJS wants to merge 1 commit into
rnmapbox:mainfrom
giaBaoJS:fix/camera-padding-partial
Open

fix(Camera): allow partial padding on the root padding prop#4279
giaBaoJS wants to merge 1 commit into
rnmapbox:mainfrom
giaBaoJS:fix/camera-padding-partial

Conversation

@giaBaoJS

Copy link
Copy Markdown

Description

Fixes #3599

CameraStop.padding is typed as CameraPadding, which declares all four edges as required. So the perfectly reasonable padding={{ paddingBottom: 10 }} fails to compile — even though the runtime has supported partial padding all along. The type was simply lying about the implementation.

buildNativeStop reads each edge independently and forwards only the ones that are actually defined (src/components/Camera.tsx#L331-L353):

const paddingTop = stop.padding?.paddingTop ?? stop.bounds?.paddingTop;
if (paddingTop !== undefined) {
  _nativeStop.paddingTop = paddingTop;
}
// ... same shape for right / bottom / left

The fix widens the prop to Partial<CameraPadding>. This matches two conventions already present in the same file:

Location Declaration
Camera.tsx:170 followPadding?: Partial<CameraPadding>
Camera.tsx:223 CameraBoundsWithPadding = Partial<CameraPadding> & CameraBounds

The repo's own example app already assumes this is how padding works — example/src/examples/UserLocation/UserLocationPadding.tsx:19 declares Record<Alignment, Partial<CameraPadding>> and passes { paddingBottom: 300 }.

The exported CameraPadding type is left untouched, so nothing that consumes it today changes meaning — this is not a breaking change, it only accepts more than it did before.

Reproduction and before/after evidence

Per .github/REPRODUCING.md. This is a compile-time bug, so the reproducer is a type-check: it needs no device, no simulator and no Mapbox token, and it fails on the unfixed build and passes on the fixed one.

The reproducer file was placed at the repo root and run through the repo's own gate, yarn typecheck. It is not committed — it is reproduced in full below.

Reproducer — repro-3599.tsx (repo root)
import React from 'react';
import { MapView, Camera } from '@rnmapbox/maps';

// --- Reproducer from issue #3599 -------------------------------------------
const BugReportExample: React.FC = () => {
  return (
    <MapView style={{ flex: 1 }}>
      <Camera padding={{ paddingBottom: 10 }} />
    </MapView>
  );
};

// A single edge other than the one in the issue, to show it is not special.
const SingleTop: React.FC = () => <Camera padding={{ paddingTop: 10 }} />;

// The empty object and the full object must both stay valid.
const Empty: React.FC = () => <Camera padding={{}} />;
const Full: React.FC = () => (
  <Camera
    padding={{ paddingTop: 1, paddingRight: 2, paddingBottom: 3, paddingLeft: 4 }}
  />
);

// --- Non-vacuity guards ----------------------------------------------------
// These MUST remain type errors. `@ts-expect-error` inverts the assertion: if any
// of them stops erroring, tsc fails with "Unused '@ts-expect-error' directive",
// so this fixture cannot pass simply because everything now compiles.

const WrongValueType: React.FC = () => (
  // @ts-expect-error - a string is not a valid padding value
  <Camera padding={{ paddingTop: '10' }} />
);

const UnknownEdge: React.FC = () => (
  // @ts-expect-error - an unknown edge key must still be rejected
  <Camera padding={{ paddingTopLeft: 10 }} />
);

const NotAnObject: React.FC = () => (
  // @ts-expect-error - a bare number is not a CameraPadding
  <Camera padding={10} />
);

export { BugReportExample, SingleTop, Empty, Full, WrongValueType, UnknownEdge, NotAnObject };

BEFORE the fixyarn typecheck, exit code 2:

repro-3599.tsx(9,15): error TS2739: Type '{ paddingBottom: number; }' is missing the following properties from type 'CameraPadding': paddingLeft, paddingRight, paddingTop
repro-3599.tsx(15,43): error TS2739: Type '{ paddingTop: number; }' is missing the following properties from type 'CameraPadding': paddingLeft, paddingRight, paddingBottom
repro-3599.tsx(18,39): error TS2739: Type '{}' is missing the following properties from type 'CameraPadding': paddingLeft, paddingRight, paddingTop, paddingBottom

That first line is exactly the error reported in #3599.

AFTER the fixyarn typecheck, exit code 0, no output.

The reproducer is not vacuous

A "before red / after green" result is only worth something if the check is capable of going red for the right reason. Two things were verified:

  1. The harness actually covers the fixture location. A deliberate error was placed at repro-3599.tsx first, and yarn typecheck did report it — so the file is genuinely compiled and not silently excluded by tsconfig.json:
    repro-3599.tsx(1,7): error TS2322: Type 'string' is not assignable to type 'number'.
    
  2. The widening did not disable checking. Temporarily removing the three @ts-expect-error directives after the fix shows the guarded cases are still real errors — only optionality changed, nothing else:
    repro-3599.tsx(37,22): error TS2322: Type 'string' is not assignable to type 'number'.
    repro-3599.tsx(42,22): error TS2561: Object literal may only specify known properties, but 'paddingTopLeft' does not exist in type 'Partial<CameraPadding>'. Did you mean to write 'paddingLeft'?
    repro-3599.tsx(47,11): error TS2559: Type 'number' has no properties in common with type 'Partial<CameraPadding>'.
    

Runtime impact

Every consumer of the prop type was checked; a partial object cannot reach a code path that assumes a complete one.

  • NativebuildNativeStop (Camera.tsx:331-353) is already per-edge !== undefined guarded, as quoted above. nativeStop (:390-421) only forwards the prop into it.
  • Websrc/web/components/Camera.tsx:170-182 gates the whole padding block on all four edges being present before calling buildMapboxGlPadding. A partial object simply leaves options.padding unset, which is already what {} does today. No partial object can reach Mapbox GL.
  • fitBounds_fitBounds (:476-525) builds a fully populated _padding literal from its own paddingConfig argument and never consumes the prop type, so widening cannot affect it.

Note on generated docs

yarn generate was run, as required. One side effect worth flagging: the doc generator cannot expand a Partial<T>, so docs/Camera.md now renders the padding prop as Partial instead of the expanded four-field shape. This is a pre-existing limitation rather than something new — followPadding already renders as bare Partial for the same reason. The JSDoc was extended to "Individual edges may be omitted." to compensate. Happy to inline the optional shape literal instead if you would rather keep the expanded table in the docs.

Verification

All run at cbf2a2d (the merge-base) with the change applied:

Gate Before After
yarn typecheck exit 0 exit 0
yarn unittest 22 suites / 112 tests passed 22 suites / 112 tests passed
yarn lint exit 0 (92 pre-existing warnings) exit 0 (92 pre-existing warnings, none in touched files)
exampleyarn type:check exit 0 exit 0
yarn generate exit 0, clean tree exit 0, clean tree

Checklist

  • I've read CONTRIBUTING.md
  • I updated the doc/other generated code with running yarn generate in the root folder
  • I have tested the new feature on /example app.
    • In V11 mode/ios
    • In New Architecture mode/ios
    • In V11 mode/android
    • In New Architecture mode/android
  • I added/updated a sample - if a new feature was implemented (/example)

This is a type-only change with no runtime diff, so it is verified by the type-level reproducer above rather than by a device run. The existing UserLocationPadding example already exercises partial padding at runtime.

Component to reproduce the issue you're fixing

<MapView style={{ flex: 1 }}>
  <Camera padding={{ paddingBottom: 10 }} />
</MapView>

`CameraStop.padding` was typed as `CameraPadding`, which requires all four
edges, so `padding={{ paddingBottom: 10 }}` failed to compile with TS2739
even though the runtime already supports partial padding: `buildNativeStop`
reads every edge individually and only forwards the ones that are defined.

Widen the prop to `Partial<CameraPadding>`, matching `followPadding` and
`CameraBoundsWithPadding` in the same file. The exported `CameraPadding`
type is unchanged, so this is not a breaking change.

Fixes rnmapbox#3599
@giaBaoJS
giaBaoJS requested a deployment to CI with Mapbox Tokens August 14, 2026 02:07 — with GitHub Actions Waiting
@giaBaoJS
giaBaoJS requested a deployment to CI with Mapbox Tokens August 14, 2026 02:07 — with GitHub Actions Waiting
@giaBaoJS
giaBaoJS requested a deployment to CI with Mapbox Tokens August 14, 2026 02:07 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: The CameraPadding lists all edges as being required even though the code allows them to be optional

1 participant