[BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 #1764
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…returning the keys #1763
MSNev
requested review from
ramthi,
lzchen,
hectorhdzg,
kryalama and
Karlie-777
MSNev
commented
Feb 18, 2022
| @@ -27,6 +26,22 @@ export class CorePerfCheckTests extends AITestClass { | |||
|
|
|||
| public registerTests() { | |||
|
|
|||
| this.testCase({ | |||
| name: "PerfChecks: isString", | |||
There was a problem hiding this comment.
Added some more perf tests as well
MSNev
commented
Feb 18, 2022
| @@ -143,7 +176,7 @@ export class CorePerfCheckTests extends AITestClass { | |||
| name: "PerfChecks: objForEachKey dynamic large (40 fields) object", | |||
| timeout: 60000, | |||
| test: () => { | |||
| let iterations = 100000; | |||
| let iterations = 90000; | |||
There was a problem hiding this comment.
Reducing iterations as the perf tests sometimes timeout on the official build environments
MSNev
commented
Feb 18, 2022
| @@ -459,7 +492,7 @@ export class CorePerfCheckTests extends AITestClass { | |||
| let checks = 0; | |||
| return this._runPerfTestAsync("baseTestObject", () => { | |||
| JSON.stringify(baseTestObject); | |||
| }, 50, iterations, 0.005).then((baseDuration) => { | |||
| }, 60, iterations, 0.005).then((baseDuration) => { | |||
There was a problem hiding this comment.
Also increasing the minimum time for the same reason -- build servers are virtualized slow beasts
MSNev
commented
Feb 18, 2022
| @@ -134,7 +134,7 @@ export class HelperFuncTests extends AITestClass { | |||
| test: () => { | |||
| let newObject = objExtend(); | |||
| Assert.ok(isObject(newObject), "The returned object is an object"); | |||
| Assert.equal(0, objKeys(newObject), "The object should have no values"); | |||
| Assert.equal(0, objKeys(newObject).length, "The object should have no values"); | |||
There was a problem hiding this comment.
This was part of the issue as objKeys was actually returning the function Object.keys and not the result.
MSNev
commented
Feb 18, 2022
| @@ -357,5 +357,21 @@ export class HelperFuncTests extends AITestClass { | |||
| Assert.equal(String.prototype, _getObjProto(new String("true")), "new String('true')"); | |||
| } | |||
| }); | |||
|
|
|||
| this.testCase({ | |||
| name: "objKeys", | |||
There was a problem hiding this comment.
Add an explicit unit test to catch this specific failure in the future.
MSNev
commented
Feb 18, 2022
| @@ -85,7 +86,7 @@ const rLeadingNumeric = /^(\d+[\w\d_$])/; | |||
| } | |||
|
|
|||
| export function objToString(obj: any) { | |||
| return ObjProto[strToString].call(obj); | |||
| return _objToString.call(obj); | |||
There was a problem hiding this comment.
minor perf / minification improvements
MSNev
commented
Feb 18, 2022
| @@ -310,21 +311,22 @@ export function strContains(value: string, search: string) { | |||
| * Check if an object is of type Date | |||
| */ | |||
| export function isDate(obj: any): obj is Date { | |||
| return !!(obj && objToString(obj) === "[object Date]"); | |||
| return !!(obj && _objToString.call(obj) === "[object Date]"); | |||
There was a problem hiding this comment.
perf improvement to avoid additional function call
MSNev
commented
Feb 18, 2022
| } | ||
|
|
||
| /** | ||
| * Check if an object is of type Array | ||
| */ | ||
| export function isArray<T = any>(obj: any): obj is Array<T> { | ||
| return _isArray ? _isArray(obj) : !!(obj && objToString(obj) === "[object Array]"); | ||
| export let isArray: <T = any>(obj: any) => obj is Array<T> = _isArray || _isArrayPoly; |
There was a problem hiding this comment.
Perf improvement to avoid an additional function call.
so now isArray will either be
- an alias to
Array.isArraywhen supported (So becomes a direct native call) - an alias to the previous polyfill function when not. (stays as the previous poly function)
MSNev
commented
Feb 18, 2022
| @@ -375,24 +377,24 @@ export function isPlainObject(value: any): boolean { | |||
| let result: boolean = false; | |||
|
|
|||
| if (value && typeof value === "object") { | |||
| let proto = _getObjProto(value); | |||
| // Inlining _objGetPrototypeOf for performance to avoid an additional function call | |||
There was a problem hiding this comment.
Lots of perf improvements, reduces the runtime (in the perf tests) locally from over 200ms for 100000 iterations to < 50ms (30ish on Chromium Edge and 40is on Firefox)
MSNev
commented
Feb 18, 2022
| @@ -598,7 +600,7 @@ export function objKeys(obj: {}): string[] { | |||
|
|
|||
| // For Performance try and use the native instance, using string lookup of the function to easily pass the ES3 build checks and minification | |||
| if (!_objKeysHasDontEnumBug && obj[strKeys]) { | |||
| return obj[strKeys]; | |||
| return obj[strKeys](); | |||
There was a problem hiding this comment.
This was the actual issue missing the () to actually call the function.
MSNev
commented
Feb 18, 2022
| export function objSeal<T>(value: T): T { | ||
| return _objSeal ? _objSeal(value) as T : value; | ||
| } | ||
| export const objFreeze: <T>(value: T) => T = _objFreeze || _doNothing; |
There was a problem hiding this comment.
More perf improvements to avoid an extra function call objFreeze() => Object.freeze().
As with isArray, these are now an aliases to the available native function or a dummy one that does nothing. This also is a minor minification improvement as well.
Karlie-777
approved these changes
Feb 18, 2022
MSNev
added a commit
that referenced
this pull request
Feb 18, 2022
…ranch (#1762) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Enable GitHub Actions on [beta] branch
MSNev
added a commit
that referenced
this pull request
Mar 3, 2022
* Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Update version.json (#1767) * [Release] Increase version to 2.7.4 (#1770) - Updates React Plugin to v3.2.4 (with v2.7.4 as dependency) - Updates React Native Plugin to 2.4.4 (with v2.7.4 as dependency) - Updates Chrome Debug Extension to 0.2.4 This release is primarily a performance improvement release where we will now use any built in (or provided polyfill) function over the internal polyfills for - String trim() - String endsWith() - String startsWith() - Additional Date toISOString() - Array isArray() - Array indexOf() - Array map() - Array reduce() - Object freeze() - Object seal() * [Beta] Keep version.json next as minor and resync shrinkwrap - Fix merge issue
MSNev
added a commit
that referenced
this pull request
Mar 31, 2022
…ster (#1791) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) (#1757) * Merge [master] branch to [beta] and Enable GitHub Actions on [beta] branch (#1762) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Enable GitHub Actions on [beta] branch * Beta Part 1: Part of Mega Dynamic Load/Unload support (#1766) - Refactor TelemetryPluginChain ready to start supporting load/unload - Move TelemetryInitializer to BaseCore - add getPlugin (will be used for remove) - Address Channel flush issue * Merge remote-tracking branch 'upstream/master' into beta (#1772) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Update version.json (#1767) * [Release] Increase version to 2.7.4 (#1770) - Updates React Plugin to v3.2.4 (with v2.7.4 as dependency) - Updates React Native Plugin to 2.4.4 (with v2.7.4 as dependency) - Updates Chrome Debug Extension to 0.2.4 This release is primarily a performance improvement release where we will now use any built in (or provided polyfill) function over the internal polyfills for - String trim() - String endsWith() - String startsWith() - Additional Date toISOString() - Array isArray() - Array indexOf() - Array map() - Array reduce() - Object freeze() - Object seal() * [Beta] Keep version.json next as minor and resync shrinkwrap - Fix merge issue * Beta Part 2: Part of Mega Dynamic Load/Unload support (#1768) - Add Event Namespace support - Minification of constant values - Add part of the unload functionality (required for unified `teardown()` functionality) * Beta Part 3: Part of Mega Dynamic Load/Unload support (#1780) * Beta Part 3: Part of Mega Dynamic Load/Unload support - Add Core SDK Unload support * Fix telemetry chain for null and undefined * Beta Part 4: Part of Mega Dynamic Load/Unload support (#1781) * Beta Part 4: Part of Mega Dynamic Load/Unload support - Fix function typing issues - Update Analytics Extension to start supporting teardown / unload (more tests required) - Adds namespace option to instrumentation hooks (for debugging teardown issues) - Update AITest Class to log and optionally assert events and hooks that have not been removed - Add Update callback when plugins are added / removed (will be extended for config updates) - Some minor minification improvements * Update comments * Add missing enum definition * Update Sender tests * Beta Part 5: Part of Mega Dynamic Load/Unload support (#1782) - Add Missing Exports - AnalyticsPlugin: Implement teardown and initial test validation - Dependencies Plugin: Implement teardown and initial test validation - Add flush() to IAppInsightsCore * AI Beta: Minor bug fixes and additional debug info (#1787) * Lint fixes: Enable Automatic formatting fixes (#1788) * Beta Part 6: Part of Mega Dynamic Load/Unload support (#1782) (#1789) - Add basic minimal unload / teardown support to all remaining components - Update rollup cleanup dependencies * Beta: Component Governance Updates to address known dependency issues (#1790)
MSNev
added a commit
that referenced
this pull request
Apr 30, 2022
…ster (#1791) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) (#1757) * Merge [master] branch to [beta] and Enable GitHub Actions on [beta] branch (#1762) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Enable GitHub Actions on [beta] branch * Beta Part 1: Part of Mega Dynamic Load/Unload support (#1766) - Refactor TelemetryPluginChain ready to start supporting load/unload - Move TelemetryInitializer to BaseCore - add getPlugin (will be used for remove) - Address Channel flush issue * Merge remote-tracking branch 'upstream/master' into beta (#1772) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Update version.json (#1767) * [Release] Increase version to 2.7.4 (#1770) - Updates React Plugin to v3.2.4 (with v2.7.4 as dependency) - Updates React Native Plugin to 2.4.4 (with v2.7.4 as dependency) - Updates Chrome Debug Extension to 0.2.4 This release is primarily a performance improvement release where we will now use any built in (or provided polyfill) function over the internal polyfills for - String trim() - String endsWith() - String startsWith() - Additional Date toISOString() - Array isArray() - Array indexOf() - Array map() - Array reduce() - Object freeze() - Object seal() * [Beta] Keep version.json next as minor and resync shrinkwrap - Fix merge issue * Beta Part 2: Part of Mega Dynamic Load/Unload support (#1768) - Add Event Namespace support - Minification of constant values - Add part of the unload functionality (required for unified `teardown()` functionality) * Beta Part 3: Part of Mega Dynamic Load/Unload support (#1780) * Beta Part 3: Part of Mega Dynamic Load/Unload support - Add Core SDK Unload support * Fix telemetry chain for null and undefined * Beta Part 4: Part of Mega Dynamic Load/Unload support (#1781) * Beta Part 4: Part of Mega Dynamic Load/Unload support - Fix function typing issues - Update Analytics Extension to start supporting teardown / unload (more tests required) - Adds namespace option to instrumentation hooks (for debugging teardown issues) - Update AITest Class to log and optionally assert events and hooks that have not been removed - Add Update callback when plugins are added / removed (will be extended for config updates) - Some minor minification improvements * Update comments * Add missing enum definition * Update Sender tests * Beta Part 5: Part of Mega Dynamic Load/Unload support (#1782) - Add Missing Exports - AnalyticsPlugin: Implement teardown and initial test validation - Dependencies Plugin: Implement teardown and initial test validation - Add flush() to IAppInsightsCore * AI Beta: Minor bug fixes and additional debug info (#1787) * Lint fixes: Enable Automatic formatting fixes (#1788) * Beta Part 6: Part of Mega Dynamic Load/Unload support (#1782) (#1789) - Add basic minimal unload / teardown support to all remaining components - Update rollup cleanup dependencies * Beta: Component Governance Updates to address known dependency issues (#1790)
MSNev
added a commit
that referenced
this pull request
Apr 30, 2022
* Update version.json (#1767) * [Release] Increase version to 2.7.4 (#1770) - Updates React Plugin to v3.2.4 (with v2.7.4 as dependency) - Updates React Native Plugin to 2.4.4 (with v2.7.4 as dependency) - Updates Chrome Debug Extension to 0.2.4 This release is primarily a performance improvement release where we will now use any built in (or provided polyfill) function over the internal polyfills for - String trim() - String endsWith() - String startsWith() - Additional Date toISOString() - Array isArray() - Array indexOf() - Array map() - Array reduce() - Object freeze() - Object seal() * remove article link in the ReadMe (#1771) * remove article links * fix readme error * Add custom properties argument to useTrackMetric (#1779) * Merge remote-tracking branch 'upstream/beta' into MSNev/MergeBetaToMaster (#1791) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) (#1757) * Merge [master] branch to [beta] and Enable GitHub Actions on [beta] branch (#1762) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Enable GitHub Actions on [beta] branch * Beta Part 1: Part of Mega Dynamic Load/Unload support (#1766) - Refactor TelemetryPluginChain ready to start supporting load/unload - Move TelemetryInitializer to BaseCore - add getPlugin (will be used for remove) - Address Channel flush issue * Merge remote-tracking branch 'upstream/master' into beta (#1772) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Update version.json (#1767) * [Release] Increase version to 2.7.4 (#1770) - Updates React Plugin to v3.2.4 (with v2.7.4 as dependency) - Updates React Native Plugin to 2.4.4 (with v2.7.4 as dependency) - Updates Chrome Debug Extension to 0.2.4 This release is primarily a performance improvement release where we will now use any built in (or provided polyfill) function over the internal polyfills for - String trim() - String endsWith() - String startsWith() - Additional Date toISOString() - Array isArray() - Array indexOf() - Array map() - Array reduce() - Object freeze() - Object seal() * [Beta] Keep version.json next as minor and resync shrinkwrap - Fix merge issue * Beta Part 2: Part of Mega Dynamic Load/Unload support (#1768) - Add Event Namespace support - Minification of constant values - Add part of the unload functionality (required for unified `teardown()` functionality) * Beta Part 3: Part of Mega Dynamic Load/Unload support (#1780) * Beta Part 3: Part of Mega Dynamic Load/Unload support - Add Core SDK Unload support * Fix telemetry chain for null and undefined * Beta Part 4: Part of Mega Dynamic Load/Unload support (#1781) * Beta Part 4: Part of Mega Dynamic Load/Unload support - Fix function typing issues - Update Analytics Extension to start supporting teardown / unload (more tests required) - Adds namespace option to instrumentation hooks (for debugging teardown issues) - Update AITest Class to log and optionally assert events and hooks that have not been removed - Add Update callback when plugins are added / removed (will be extended for config updates) - Some minor minification improvements * Update comments * Add missing enum definition * Update Sender tests * Beta Part 5: Part of Mega Dynamic Load/Unload support (#1782) - Add Missing Exports - AnalyticsPlugin: Implement teardown and initial test validation - Dependencies Plugin: Implement teardown and initial test validation - Add flush() to IAppInsightsCore * AI Beta: Minor bug fixes and additional debug info (#1787) * Lint fixes: Enable Automatic formatting fixes (#1788) * Beta Part 6: Part of Mega Dynamic Load/Unload support (#1782) (#1789) - Add basic minimal unload / teardown support to all remaining components - Update rollup cleanup dependencies * Beta: Component Governance Updates to address known dependency issues (#1790) * Master Minification Improvements (#1793) * Master Minification Improvements - Remove Generated classes - And Convert to interfaces rather than classes * Update version.json so next is a minor release * Minification - Change to only use const enums internally (#1796) - Remove Angular config from CodeQL * More Common Minification Updates (#1798) - SeverityLevel => const enum - RequestHeaders - TraceParent * Enable fetch automatic dependency tracking by default (#1468) * Finalize and Update the processTelemetry helper functions (#1805) * [Release] Increase version to v2.8.0 (#1806) * [BUG] Adding new on-by-default telemetry is a breaking change, and should involve a major version bump #1808 (#1809) * v2.8.0 has incompatible TypeScript 3.x type declaration (#1810) - [BUG] Angular project doesn't build after install latest version v.2.8.0 #1807 * [BUG] Browser exceptions are no longer automatically tracked after 2.8.0 #1812 (#1815) * [Release] Increase version to 2.8.1 (#1816) * [BUG] v2.8.1 with a Hosted IE environment fails to initialize for a hosted instance of IE #1822 (#1824) [BUG] IE8 Support was broken by several components #1823 * [Release] Increase version to 2.7.4 (#1770) - Updates React Plugin to v3.2.4 (with v2.7.4 as dependency) - Updates React Native Plugin to 2.4.4 (with v2.7.4 as dependency) - Updates Chrome Debug Extension to 0.2.4 This release is primarily a performance improvement release where we will now use any built in (or provided polyfill) function over the internal polyfills for - String trim() - String endsWith() - String startsWith() - Additional Date toISOString() - Array isArray() - Array indexOf() - Array map() - Array reduce() - Object freeze() - Object seal() * Merge remote-tracking branch 'upstream/beta' into MSNev/MergeBetaToMaster (#1791) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) (#1757) * Merge [master] branch to [beta] and Enable GitHub Actions on [beta] branch (#1762) * Update version update script to support default "next" release version (major/minor) not just patch (#1756) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Enable GitHub Actions on [beta] branch * Beta Part 1: Part of Mega Dynamic Load/Unload support (#1766) - Refactor TelemetryPluginChain ready to start supporting load/unload - Move TelemetryInitializer to BaseCore - add getPlugin (will be used for remove) - Address Channel flush issue * Merge remote-tracking branch 'upstream/master' into beta (#1772) * Additional Performance enhancements to use provided functions rather than internal polyfill's (#1758) * [BUG] 2.7.4-nightly.2202-03 builds have a bug where objKeys() is not returning the keys #1763 (#1764) * Update version.json (#1767) * [Release] Increase version to 2.7.4 (#1770) - Updates React Plugin to v3.2.4 (with v2.7.4 as dependency) - Updates React Native Plugin to 2.4.4 (with v2.7.4 as dependency) - Updates Chrome Debug Extension to 0.2.4 This release is primarily a performance improvement release where we will now use any built in (or provided polyfill) function over the internal polyfills for - String trim() - String endsWith() - String startsWith() - Additional Date toISOString() - Array isArray() - Array indexOf() - Array map() - Array reduce() - Object freeze() - Object seal() * [Beta] Keep version.json next as minor and resync shrinkwrap - Fix merge issue * Beta Part 2: Part of Mega Dynamic Load/Unload support (#1768) - Add Event Namespace support - Minification of constant values - Add part of the unload functionality (required for unified `teardown()` functionality) * Beta Part 3: Part of Mega Dynamic Load/Unload support (#1780) * Beta Part 3: Part of Mega Dynamic Load/Unload support - Add Core SDK Unload support * Fix telemetry chain for null and undefined * Beta Part 4: Part of Mega Dynamic Load/Unload support (#1781) * Beta Part 4: Part of Mega Dynamic Load/Unload support - Fix function typing issues - Update Analytics Extension to start supporting teardown / unload (more tests required) - Adds namespace option to instrumentation hooks (for debugging teardown issues) - Update AITest Class to log and optionally assert events and hooks that have not been removed - Add Update callback when plugins are added / removed (will be extended for config updates) - Some minor minification improvements * Update comments * Add missing enum definition * Update Sender tests * Beta Part 5: Part of Mega Dynamic Load/Unload support (#1782) - Add Missing Exports - AnalyticsPlugin: Implement teardown and initial test validation - Dependencies Plugin: Implement teardown and initial test validation - Add flush() to IAppInsightsCore * AI Beta: Minor bug fixes and additional debug info (#1787) * Lint fixes: Enable Automatic formatting fixes (#1788) * Beta Part 6: Part of Mega Dynamic Load/Unload support (#1782) (#1789) - Add basic minimal unload / teardown support to all remaining components - Update rollup cleanup dependencies * Beta: Component Governance Updates to address known dependency issues (#1790) * Master Minification Improvements (#1793) * Master Minification Improvements - Remove Generated classes - And Convert to interfaces rather than classes * Update version.json so next is a minor release * Minification - Change to only use const enums internally (#1796) - Remove Angular config from CodeQL * More Common Minification Updates (#1798) - SeverityLevel => const enum - RequestHeaders - TraceParent * Enable fetch automatic dependency tracking by default (#1468) * Finalize and Update the processTelemetry helper functions (#1805) * [Release] Increase version to v2.8.0 (#1806) * [BUG] Adding new on-by-default telemetry is a breaking change, and should involve a major version bump #1808 (#1809) * v2.8.0 has incompatible TypeScript 3.x type declaration (#1810) - [BUG] Angular project doesn't build after install latest version v.2.8.0 #1807 * [BUG] Browser exceptions are no longer automatically tracked after 2.8.0 #1812 (#1815) * [Release] Increase version to 2.8.1 (#1816) * Fix merge Issues -- full compare with master - Update version.json so next beta will be 3.0.0 * Set next beta release as major Co-authored-by: Karlie-777 <79606506+Karlie-777@users.noreply.github.com> Co-authored-by: Simo Nurmi <simo.aleksi.nurmi@gmail.com> Co-authored-by: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.