Skip to content

Commit a950634

Browse files
wangqingyangfacebook-github-bot
wangqingyang
authored andcommitted
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 <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/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: #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
1 parent 4f0671b commit a950634

File tree

2 files changed

+38
-18
lines changed

2 files changed

+38
-18
lines changed

Libraries/LogBox/Data/LogBoxData.js

+18-18
Original file line numberDiff line numberDiff line change
@@ -320,40 +320,40 @@ export function checkWarningFilter(format: string): WarningInfo {
320320
return warningFilter(format);
321321
}
322322

323+
export function getIgnorePatterns(): $ReadOnlyArray<IgnorePattern> {
324+
return Array.from(ignorePatterns);
325+
}
326+
323327
export function addIgnorePatterns(
324328
patterns: $ReadOnlyArray<IgnorePattern>,
325329
): void {
330+
const existingSize = ignorePatterns.size;
326331
// The same pattern may be added multiple times, but adding a new pattern
327332
// can be expensive so let's find only the ones that are new.
328-
const newPatterns = patterns.filter((pattern: IgnorePattern) => {
333+
patterns.forEach((pattern: IgnorePattern) => {
329334
if (pattern instanceof RegExp) {
330-
for (const existingPattern of ignorePatterns.entries()) {
335+
for (const existingPattern of ignorePatterns) {
331336
if (
332337
existingPattern instanceof RegExp &&
333338
existingPattern.toString() === pattern.toString()
334339
) {
335-
return false;
340+
return;
336341
}
337342
}
338-
return true;
343+
ignorePatterns.add(pattern);
339344
}
340-
return !ignorePatterns.has(pattern);
345+
ignorePatterns.add(pattern);
341346
});
342-
343-
if (newPatterns.length === 0) {
347+
if (ignorePatterns.size === existingSize) {
344348
return;
345349
}
346-
for (const pattern of newPatterns) {
347-
ignorePatterns.add(pattern);
348-
349-
// We need to recheck all of the existing logs.
350-
// This allows adding an ignore pattern anywhere in the codebase.
351-
// Without this, if you ignore a pattern after the a log is created,
352-
// then we would keep showing the log.
353-
logs = new Set(
354-
Array.from(logs).filter(log => !isMessageIgnored(log.message.content)),
355-
);
356-
}
350+
// We need to recheck all of the existing logs.
351+
// This allows adding an ignore pattern anywhere in the codebase.
352+
// Without this, if you ignore a pattern after the a log is created,
353+
// then we would keep showing the log.
354+
logs = new Set(
355+
Array.from(logs).filter(log => !isMessageIgnored(log.message.content)),
356+
);
357357
handleUpdate();
358358
}
359359

Libraries/LogBox/Data/__tests__/LogBoxData-test.js

+20
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,26 @@ describe('LogBoxData', () => {
418418
expect(logs[0].count).toBe(2);
419419
});
420420

421+
it('adding same pattern multiple times', () => {
422+
expect(LogBoxData.getIgnorePatterns().length).toBe(0);
423+
LogBoxData.addIgnorePatterns(['abc']);
424+
expect(LogBoxData.getIgnorePatterns().length).toBe(1);
425+
LogBoxData.addIgnorePatterns([/abc/]);
426+
expect(LogBoxData.getIgnorePatterns().length).toBe(2);
427+
LogBoxData.addIgnorePatterns(['abc']);
428+
expect(LogBoxData.getIgnorePatterns().length).toBe(2);
429+
LogBoxData.addIgnorePatterns([/abc/]);
430+
expect(LogBoxData.getIgnorePatterns().length).toBe(2);
431+
});
432+
433+
it('adding duplicated patterns', () => {
434+
expect(LogBoxData.getIgnorePatterns().length).toBe(0);
435+
LogBoxData.addIgnorePatterns(['abc', /ab/, /abc/, /abc/, 'abc']);
436+
expect(LogBoxData.getIgnorePatterns().length).toBe(3);
437+
LogBoxData.addIgnorePatterns([/ab/, /abc/]);
438+
expect(LogBoxData.getIgnorePatterns().length).toBe(3);
439+
});
440+
421441
it('ignores logs matching patterns (logs)', () => {
422442
addLogs(['A!', 'B?', 'C!']);
423443

0 commit comments

Comments
 (0)