Skip to content

fix: Ignoring result from pure array method#13632

Open
odaysec wants to merge 1 commit intomapbox:mainfrom
odaysec:patch-1
Open

fix: Ignoring result from pure array method#13632
odaysec wants to merge 1 commit intomapbox:mainfrom
odaysec:patch-1

Conversation

@odaysec
Copy link

@odaysec odaysec commented Mar 9, 2026

fix issue must actually incorporate this.scope into the res array when serializing. Since Array.prototype.concat returns a new array, we should either assign that result back to res or, more simply, use res.push(this.scope) which mutates res in place. This maintains existing functionality and type shape (an array starting with "config" and the key, optionally followed by scope).

The best minimal fix is within Config.serialize in src/style-spec/expression/definitions/config.ts: change the line res.concat(this.scope); to res.push(this.scope);. This requires no new imports or additional helper methods. Alternatively, res = res.concat(this.scope); would also work, but push is simpler and clearer that res is being mutated. No other parts of the file need to be changed.

Specifically:

  • In serialize(), at the if (this.scope) { ... } block, replace the concat call with push.
  • Ensure the function still returns res as before.

Launch Checklist

  • Make sure the PR title is descriptive and preferably reflects the change from the user's perspective.
  • Add additional detail and context in the PR description (with screenshots/videos if there are visual changes).
  • Manually test the debug page.
  • Write tests for all new functionality and make sure the CI checks pass.
  • Document any changes to public APIs.
  • Post benchmark scores if the change could affect performance.
  • Tag @mapbox/map-design-team @mapbox/static-apis if this PR includes style spec API or visual changes.
  • Tag @mapbox/gl-native if this PR includes shader changes or needs a native port.
  • Tag @mapbox/gl-native if this PR disables any test because it also needs to be disabled on their side.
  • Create a ticket for gl-native to groom in the MAPSNAT JIRA queue if this PR includes shader changes or features not present in the native side or if it disables a test that's not disabled there.

@odaysec odaysec requested a review from a team as a code owner March 9, 2026 06:04
@github-actions
Copy link

github-actions bot commented Mar 9, 2026

Hey, @odaysec 👋 Thanks for your contribution to Mapbox GL JS!

Important: This repository does not accept direct merges. All changes go through our internal review process.

What happens next:

  1. A team member will review your PR here first
  2. If it looks good, they will import it to our internal repository for further review
  3. If approved, changes will be synced back here via our release process

Please respond to any review comments on this PR. For more details, see CONTRIBUTING.md.

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.

1 participant