From a950634424cddf31c0adb6c9799adf1cc5f83bf0 Mon Sep 17 00:00:00 2001 From: wangqingyang Date: Tue, 7 Sep 2021 11:59:09 -0700 Subject: [PATCH] fix: compare the LogBoxData ignorePatterns with the right code (#31977) Summary: the `LogBoxData.addIgnorePatterns` function shows the wrong code to get a item in `Set`. because every item in `set.entries` looks like `[value, value]`, but not `value`. while the `addIgnorePatterns` function evalutes two itertaions that is not necessary. So I refacted this function. ## Changelog [General] [Fixed] - for LogBox checking existingPattern in a wrong way. [General] [Changed] - addIgnorePatterns runs in one iteration. [General] [Added] - add a function `getIgnorePatterns` in `LogBoxData.js` for tests or other usecases. Pull Request resolved: https://github.com/facebook/react-native/pull/31977 Test Plan: test codes in `LogBoxData-test.js`: ````js it('adding same pattern multiple times', () => { expect(LogBoxData.getIgnorePatterns().length).toBe(0); LogBoxData.addIgnorePatterns(['abc']); expect(LogBoxData.getIgnorePatterns().length).toBe(1); LogBoxData.addIgnorePatterns([/abc/]); expect(LogBoxData.getIgnorePatterns().length).toBe(2); LogBoxData.addIgnorePatterns(['abc']); expect(LogBoxData.getIgnorePatterns().length).toBe(2); LogBoxData.addIgnorePatterns([/abc/]); expect(LogBoxData.getIgnorePatterns().length).toBe(2); }); it('adding duplicated patterns', () => { expect(LogBoxData.getIgnorePatterns().length).toBe(0); LogBoxData.addIgnorePatterns(['abc', /ab/, /abc/, /abc/, 'abc']); expect(LogBoxData.getIgnorePatterns().length).toBe(3); LogBoxData.addIgnorePatterns([/ab/, /abc/]); expect(LogBoxData.getIgnorePatterns().length).toBe(3); }); ```` and they have passed Reviewed By: rickhanlonii Differential Revision: D30675522 Pulled By: yungsters fbshipit-source-id: 4a05e0f04a41d06cac416219f1e8e540bf0eea02 --- Libraries/LogBox/Data/LogBoxData.js | 36 +++++++++---------- .../LogBox/Data/__tests__/LogBoxData-test.js | 20 +++++++++++ 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js index 04681c2814273c..55d8dd4165a3ed 100644 --- a/Libraries/LogBox/Data/LogBoxData.js +++ b/Libraries/LogBox/Data/LogBoxData.js @@ -320,40 +320,40 @@ export function checkWarningFilter(format: string): WarningInfo { return warningFilter(format); } +export function getIgnorePatterns(): $ReadOnlyArray { + return Array.from(ignorePatterns); +} + export function addIgnorePatterns( patterns: $ReadOnlyArray, ): void { + const existingSize = ignorePatterns.size; // The same pattern may be added multiple times, but adding a new pattern // can be expensive so let's find only the ones that are new. - const newPatterns = patterns.filter((pattern: IgnorePattern) => { + patterns.forEach((pattern: IgnorePattern) => { if (pattern instanceof RegExp) { - for (const existingPattern of ignorePatterns.entries()) { + for (const existingPattern of ignorePatterns) { if ( existingPattern instanceof RegExp && existingPattern.toString() === pattern.toString() ) { - return false; + return; } } - return true; + ignorePatterns.add(pattern); } - return !ignorePatterns.has(pattern); + ignorePatterns.add(pattern); }); - - if (newPatterns.length === 0) { + if (ignorePatterns.size === existingSize) { return; } - for (const pattern of newPatterns) { - ignorePatterns.add(pattern); - - // We need to recheck all of the existing logs. - // This allows adding an ignore pattern anywhere in the codebase. - // Without this, if you ignore a pattern after the a log is created, - // then we would keep showing the log. - logs = new Set( - Array.from(logs).filter(log => !isMessageIgnored(log.message.content)), - ); - } + // We need to recheck all of the existing logs. + // This allows adding an ignore pattern anywhere in the codebase. + // Without this, if you ignore a pattern after the a log is created, + // then we would keep showing the log. + logs = new Set( + Array.from(logs).filter(log => !isMessageIgnored(log.message.content)), + ); handleUpdate(); } diff --git a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js index 3cc4ac2a9877b1..5e4225e946560b 100644 --- a/Libraries/LogBox/Data/__tests__/LogBoxData-test.js +++ b/Libraries/LogBox/Data/__tests__/LogBoxData-test.js @@ -418,6 +418,26 @@ describe('LogBoxData', () => { expect(logs[0].count).toBe(2); }); + it('adding same pattern multiple times', () => { + expect(LogBoxData.getIgnorePatterns().length).toBe(0); + LogBoxData.addIgnorePatterns(['abc']); + expect(LogBoxData.getIgnorePatterns().length).toBe(1); + LogBoxData.addIgnorePatterns([/abc/]); + expect(LogBoxData.getIgnorePatterns().length).toBe(2); + LogBoxData.addIgnorePatterns(['abc']); + expect(LogBoxData.getIgnorePatterns().length).toBe(2); + LogBoxData.addIgnorePatterns([/abc/]); + expect(LogBoxData.getIgnorePatterns().length).toBe(2); + }); + + it('adding duplicated patterns', () => { + expect(LogBoxData.getIgnorePatterns().length).toBe(0); + LogBoxData.addIgnorePatterns(['abc', /ab/, /abc/, /abc/, 'abc']); + expect(LogBoxData.getIgnorePatterns().length).toBe(3); + LogBoxData.addIgnorePatterns([/ab/, /abc/]); + expect(LogBoxData.getIgnorePatterns().length).toBe(3); + }); + it('ignores logs matching patterns (logs)', () => { addLogs(['A!', 'B?', 'C!']);