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.

Sign up for GitHub

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

Jump to bottom

Fix map memory leak #3444

Merged
merged 9 commits into from
May 23, 2018
Merged

Fix map memory leak #3444

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
23e7629
added object-hash dependencies
May 11, 2018
7851c50
use sha1 hash to retrieve cache from maps
May 11, 2018
d8f3663
Merge branch 'master' into fix-map-memory-leak
abernix May 23, 2018
f8d1435
Merge branch 'master' into fix-map-memory-leak
abernix May 23, 2018
2c3d585
removed object-hash from dependencies
May 23, 2018
6988176
fixed package.json rebase conflicts
May 23, 2018
23f71d6
removed last object-hash cache entry
May 23, 2018
4557ca7
removed memoize/cached results tests
May 23, 2018
9f25da2
CHANGELOG update
hwillson May 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 1 addition & 30 deletions packages/apollo-utilities/src/__tests__/transform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,21 +312,6 @@ describe('query transforms', () => {

expect(expectedQueryStr).toBe(print(newQueryDoc));
});
it('should memoize and return cached results', () => {
let testQuery = gql`
query {
author {
name {
firstName
lastName
}
}
}
`;
const newQueryDoc = addTypenameToDocument(testQuery);
const secondCall = addTypenameToDocument(testQuery);
expect(newQueryDoc).toBe(secondCall);
});

it('should not add duplicates', () => {
let testQuery = gql`
Expand Down Expand Up @@ -567,21 +552,6 @@ describe('query transforms', () => {

expect(expectedQueryStr).toBe(print(newQueryDoc));
});
it('should memoize and return cached results', () => {
let testQuery = gql`
query {
author {
name @connection(key: "foo") {
firstName
lastName
}
}
}
`;
const newQueryDoc = removeConnectionDirectiveFromDocument(testQuery);
const secondCall = removeConnectionDirectiveFromDocument(testQuery);
expect(newQueryDoc).toBe(secondCall);
});
});

describe('getDirectivesFromDocument', () => {
Expand Down Expand Up @@ -982,4 +952,5 @@ describe('getDirectivesFromDocument', () => {
const doc = getDirectivesFromDocument([{ name: 'client' }], query, true);
expect(print(doc)).toBe(print(expected));
});
});
Copy link
Member

Choose a reason for hiding this comment

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

Wow, looks like the describe on line 917 wasn't closed - good catch!

Copy link
Author

@brunorzn brunorzn May 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah I had to like triple check this to be sure it wasn't me ;) (basically, eslint caught it, not me)

});
6 changes: 0 additions & 6 deletions packages/apollo-utilities/src/directives.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,7 @@ export function flattenSelections(selection: SelectionNode): SelectionNode[] {
);
}

const added = new Map();
export function getDirectiveNames(doc: DocumentNode) {
const cached = added.get(doc);
if (cached) return cached;

// operation => [names of directives];
const directives = doc.definitions
.filter(
Expand All @@ -137,8 +133,6 @@ export function getDirectiveNames(doc: DocumentNode) {
.reduce((directives, directive) => directives.concat(directive), [])
// [Directives] => [Name]
.map((directive: DirectiveNode) => directive.name.value);

added.set(doc, directives);
return directives;
}

Expand Down
14 changes: 2 additions & 12 deletions packages/apollo-utilities/src/transform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,8 @@ export function removeDirectivesFromDocument(
return isNotEmpty(operation, fragments) ? docClone : null;
}

const added = new Map();
export function addTypenameToDocument(doc: DocumentNode) {
checkDocument(doc);
const cached = added.get(doc);
if (cached) return cached;

const docClone = cloneDeep(doc);

docClone.definitions.forEach((definition: DefinitionNode) => {
Expand All @@ -180,8 +176,6 @@ export function addTypenameToDocument(doc: DocumentNode) {
isRoot,
);
});

added.set(doc, docClone);
return docClone;
}

Expand All @@ -203,14 +197,10 @@ const connectionRemoveConfig = {
return willRemove;
},
};
const removed = new Map();

export function removeConnectionDirectiveFromDocument(doc: DocumentNode) {
checkDocument(doc);
const cached = removed.get(doc);
if (cached) return cached;
const docClone = removeDirectivesFromDocument([connectionRemoveConfig], doc);
removed.set(doc, docClone);
return docClone;
return removeDirectivesFromDocument([connectionRemoveConfig], doc);
}

export type GetDirectiveConfig = {
Expand Down