Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PLAT-8735] Preserve feature flag order #1802

Merged
merged 3 commits into from
Sep 2, 2022

Conversation

gingerbenw
Copy link
Member

@gingerbenw gingerbenw commented Aug 12, 2022

Goal

To preserve the order of feature flags as they were added

Design

Changed internal feature storage from an object to an array, added relevant poly fills for IE11 compatibility

Testing

Unit and integration tests updated

@gingerbenw gingerbenw changed the title [PLATlat 8735 preserve feature flag order [PLAT-8735] Preserve feature flag order Aug 12, 2022
@github-actions
Copy link

github-actions bot commented Aug 12, 2022

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 42.67 kB 13.08 kB
After 43.13 kB 13.19 kB
± ⚠️ +460 bytes ⚠️ +115 bytes

code coverage diff

Ok File Lines Branches Functions Statements
/home/runner/work/bugsnag-js/bugsnag-js/packages/core/client.js 99.39%
(+0.01%)
97.65%
(+0%)
100%
(+0%)
98.87%
(+0.01%)
/home/runner/work/bugsnag-js/bugsnag-js/packages/core/event.js 96.03%
(+0.06%)
93.15%
(+0%)
100%
(+0%)
96.3%
(+0.06%)
/home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-ipc/bugsnag-ipc-main.js 83.08%
(+0.27%)
73.08%
(+0%)
89.47%
(+0%)
81.94%
(+0.25%)
/home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-renderer-event-data/renderer-event-data.js 90.48%
(+0.48%)
81.25%
(+0%)
100%
(+0%)
91.3%
(+0.39%)

Total:

Lines Branches Functions Statements
87.27%(+0.05%) 75.52%(+0.05%) 86.76%(+0%) 86.2%(+0.05%)

Generated by 🚫 dangerJS against aa5d206

@gingerbenw gingerbenw force-pushed the PLAT-8735-preserve-feature-flag-order branch from c1bc6bd to 68bd1c3 Compare August 18, 2022 13:34
@gingerbenw gingerbenw force-pushed the PLAT-8735-preserve-feature-flag-order branch from 68bd1c3 to eabc08c Compare August 18, 2022 13:49
for (var i = 0; i < existingFeatures.length; i++) {
if (existingFeatures[i].name === name) {
found = true
existingFeatures[i].variant = variant
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be safer to replace rather than mutate the object, since an event.features may also contain it?

e.g.

Suggested change
existingFeatures[i].variant = variant
existingFeatures[i].variant = { name, variant }

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it makes any difference here, as the flag will have been cloned from the event, rather than referenced.

@gingerbenw gingerbenw force-pushed the PLAT-8735-preserve-feature-flag-order branch from 555f060 to 62a27df Compare August 30, 2022 13:15
@gingerbenw gingerbenw merged commit 88a2e3d into next Sep 2, 2022
@gingerbenw gingerbenw mentioned this pull request Sep 8, 2022
@gingerbenw gingerbenw deleted the PLAT-8735-preserve-feature-flag-order branch September 29, 2022 14:51
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.

5 participants