Skip to content

chore: address post-merge NUT review comments (W-23400737)#677

Open
jhefferman-sfdc wants to merge 1 commit into
mainfrom
jhefferman/W-23400737-nut-review-followup
Open

chore: address post-merge NUT review comments (W-23400737)#677
jhefferman-sfdc wants to merge 1 commit into
mainfrom
jhefferman/W-23400737-nut-review-followup

Conversation

@jhefferman-sfdc

Copy link
Copy Markdown
Collaborator

What & why

Follow-up to #676 (W-23400737), addressing the two actionable review comments left after that PR merged:

  1. getAccessToken no longer exported (browserUtils.ts:31) — per @wjhsf, it's only referenced within browserUtils.ts (by getPreview), so the export was unnecessary.
  2. Number.parseInt → global parseInt (sessionUtils.ts:28) — per @wjhsf, for consistency with the rest of the repo, which uses global parseInt/parseFloat throughout (src/configMeta.ts, src/shared/previewUtils.ts, and devServerUtils.ts in this same helper dir).

Both are test-helper-only, no behavior change. Lint + tsc + pre-commit hooks pass locally.

Not addressed (already resolved on #676)

  • @rax-it's "should we try a default version?" on getMaxSupportedApiVersion was answered inline and the PR was subsequently approved — fail-fast is intentional (an empty apiVersionMetadata means the plugin is misconfigured; the shipping dependencyLoader.ts reads the same map and would break too, and there's no canonical default version constant to fall back to).
  • @wjhsf's design question ("what happens when the platform gets a new default API version but this package isn't updated?") is exactly the drift this fix is designed to absorb: the tests pin the preview to the plugin's own max supported version rather than the org's, so they stay green until a human adds the new version to apiVersionMetadata (+ its @lwc/sfdx-local-dev-dist-<v> dist). Adding real support for a newer API version remains a separate product task, by design.

@W-23400737@

Follow-up to #676:
- browserUtils: make getAccessToken non-exported (only used within the file)
- sessionUtils: use global parseInt for consistency with the rest of the repo
  (src/configMeta.ts, src/shared/previewUtils.ts, devServerUtils.ts)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jhefferman-sfdc
jhefferman-sfdc requested review from a team as code owners July 23, 2026 15:46
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.

2 participants