Skip to content

fix(net): join array fieldlist in net.retrieve so the documented usage works#475

Closed
james-enperso wants to merge 1 commit into
forcedotcom:devfrom
james-enperso:fix/net-retrieve-array-fieldlist
Closed

fix(net): join array fieldlist in net.retrieve so the documented usage works#475
james-enperso wants to merge 1 commit into
forcedotcom:devfrom
james-enperso:fix/net-retrieve-array-fieldlist

Conversation

@james-enperso

Copy link
Copy Markdown
Contributor

Problem

net.retrieve's public, typed, and documented signature takes fieldlist: string[]:

retrieve('Contact', id, ['FirstName', 'LastName'], onSuccess, onError)

…but the array was passed straight through to the native bridge as the fields query param:

const fields = fieldlist ? { fields: fieldlist } : null;

On Android the array is URL-encoded as its Java ArrayList.toString()fields=%5BFirstName%2C+LastName%5D (i.e. fields=[FirstName, LastName]), which Salesforce rejects with INVALID_FIELD / 400; iOS can't produce a valid comma-joined value from an NSArray either. So the documented/typed usage deterministically fails, and the only form that works — a pre-joined string — contradicts the TypeScript signature. The shared net.test.js only ever passes a pre-joined string, so the typed path was never exercised.

Fix

Join the array before it crosses the bridge; a string is passed through unchanged for the legacy pre-joined caller:

const fields = fieldlist
  ? { fields: Array.isArray(fieldlist) ? fieldlist.join(",") : fieldlist }
  : null;

Note on dist/ and the build

main points at dist/index.js, so the fix must ship in dist/ too. However, npm run build (tsc --build) extends @react-native/typescript-config, which sets noEmit: true — so it type-checks only and does not regenerate dist/. The committed dist/ is CommonJS produced by a separate release-time config; a naive tsc would re-emit the tree as ESM and break the package. So dist/react.force.net.js is hand-synced here to mirror the compiled src change (the .d.ts is unchanged — the signature is the same). Flagging the noEmit build discrepancy for a maintainer to look at separately; a maintainer may prefer to regenerate dist/ with the real release tooling.

Test plan

  • Add a shared-suite assertion that calls net.retrieve with a string[] fieldlist (currently only the pre-joined-string form is tested).
  • Verify a retrieve with ['FirstName','LastName'] returns the fields on both platforms.

🤖 Generated with Claude Code

https://claude.ai/code/session_015CGbVaRKrx7DeurX81NWdQ

…e works

net.retrieve's typed/documented signature takes `fieldlist: string[]`, but the
array was passed straight through to the bridge as the `fields` query param. On
Android it is URL-encoded as its Array toString ("[Name, Industry]") and rejected
by Salesforce (INVALID_FIELD / 400); iOS cannot produce a valid comma-joined value
from an NSArray either. So the documented/typed call form deterministically fails,
and the only form that works (a pre-joined string) contradicts the TypeScript type.
The shared net test dodges this by passing a pre-joined string.

Join the array before it crosses the bridge; a string is passed through unchanged
for the legacy pre-joined caller.

Note on dist/: `npm run build` (tsc --build) extends @react-native/typescript-config,
which sets noEmit:true, so it type-checks only and does NOT regenerate dist/. The
committed dist/ is CommonJS produced by a separate release-time config; a naive tsc
would re-emit it as ESM and break the package. dist/react.force.net.js is therefore
hand-synced here to mirror the compiled src change (the .d.ts is unchanged — the
signature is the same). The noEmit build discrepancy is worth a maintainer's look
separately.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015CGbVaRKrx7DeurX81NWdQ
@wmathurin

Copy link
Copy Markdown
Contributor

Hi @james-enperso — thank you for this! The bug diagnosis is spot-on, and flagging the noEmit build discrepancy was exactly the right call.

We're closing this in favour of #477, which fixes the same retrieve bug but also properly resolves the dist/ generation problem you identified rather than hand-syncing:

  • tsconfig.build.json — a new config that overrides noEmit: false / module: commonjs / moduleResolution: node, so dist/ can be regenerated reproducibly with tsc --project tsconfig.build.json by anyone.
  • setversion.sh — updated to call that config at release time, so the release process never needs a hand-edited dist/ again.
  • The dist/ in fix(net): join array fieldlist in net.retrieve so the documented usage works #477 was generated by the compiler, not written by hand.

Your contribution is credited in the commit message. Thanks again for the thorough write-up!

@wmathurin wmathurin closed this Jul 6, 2026
wmathurin added a commit that referenced this pull request Jul 7, 2026
…e works

The documented/typed signature passes fieldlist as string[], but the raw
array was forwarded straight to the native bridge as the `fields` query
param. On Android, ArrayList.toString() produces "[FirstName, LastName]";
on iOS, NSArray's description gives a similar bracketed form — both are
rejected by Salesforce with INVALID_FIELD / 400. The only working form
(a pre-joined string) contradicted the TypeScript signature.

Fix: join the array to a comma-separated string before the bridge crossing.
A string is passed through unchanged for backward compatibility with callers
that pre-join before passing.

Also adds tsconfig.build.json so dist/ can be regenerated reproducibly via
`tsc --project tsconfig.build.json` (the base tsconfig sets noEmit:true for
type-checking only). setversion.sh updated to use this config, closing the
gap where dist/ was previously hand-synced or relied on a separate
unreproducible release-time step. The dist/ regeneration in this commit also
picks up TypeScript version drift (null-coalescing rewrite, class field
declarations) — functionally identical changes.

Credit to @james-enperso for identifying the bug and the noEmit build
discrepancy in PR #475.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants