Skip to content

Commit

Permalink
fix: compare the LogBoxData ignorePatterns with the right code (#31977)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
wangqingyang authored and Luna Wei committed Nov 4, 2021
1 parent 7382f55 commit 9d601e4
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 18 deletions.
36 changes: 18 additions & 18 deletions Libraries/LogBox/Data/LogBoxData.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,40 +320,40 @@ export function checkWarningFilter(format: string): WarningInfo {
return warningFilter(format);
}

export function getIgnorePatterns(): $ReadOnlyArray<IgnorePattern> {
return Array.from(ignorePatterns);
}

export function addIgnorePatterns(
patterns: $ReadOnlyArray<IgnorePattern>,
): 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();
}

Expand Down
20 changes: 20 additions & 0 deletions Libraries/LogBox/Data/__tests__/LogBoxData-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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!']);

Expand Down

0 comments on commit 9d601e4

Please sign in to comment.