From e72a336e9b62174c77be79ff6252fb6d780dd238 Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Tue, 29 Jan 2019 05:45:05 -0500 Subject: [PATCH] fix #1266 by moving closure creation out of parsing scope (#1275) --- CHANGELOG.md | 9 +++++++++ docs/rules/no-deprecated.md | 4 ---- src/ExportMap.js | 28 +++++++++++++++++++++------- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a0a3f5ac..f21cfeb2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com). ## [Unreleased] +### Added +- `typescript` config ([#1257], thanks [@kirill-konshin]) +### Fixed +- Memory leak of `SourceCode` objects for all parsed dependencies, resolved. (issue [#1266], thanks [@asapach] and [@sergei-startsev] for digging in) ## [2.15.0] - 2019-01-22 ### Added @@ -506,6 +510,7 @@ for info on changes for earlier releases. [`memo-parser`]: ./memo-parser/README.md +[#1257]: https://github.com/benmosher/eslint-plugin-import/pull/1257 [#1232]: https://github.com/benmosher/eslint-plugin-import/pull/1232 [#1176]: https://github.com/benmosher/eslint-plugin-import/pull/1176 [#1163]: https://github.com/benmosher/eslint-plugin-import/pull/1163 @@ -595,6 +600,7 @@ for info on changes for earlier releases. [#314]: https://github.com/benmosher/eslint-plugin-import/pull/314 [#912]: https://github.com/benmosher/eslint-plugin-import/pull/912 +[#1266]: https://github.com/benmosher/eslint-plugin-import/issues/1266 [#1175]: https://github.com/benmosher/eslint-plugin-import/issues/1175 [#1058]: https://github.com/benmosher/eslint-plugin-import/issues/1058 [#931]: https://github.com/benmosher/eslint-plugin-import/issues/931 @@ -791,3 +797,6 @@ for info on changes for earlier releases. [@pzhine]: https://github.com/pzhine [@st-sloth]: https://github.com/st-sloth [@ljqx]: https://github.com/ljqx +[@kirill-konshin]: https://github.com/kirill-konshin +[@asapach]: https://github.com/asapach +[@sergei-startsev]: https://github.com/sergei-startsev diff --git a/docs/rules/no-deprecated.md b/docs/rules/no-deprecated.md index 7583651f3..fae7d8daf 100644 --- a/docs/rules/no-deprecated.md +++ b/docs/rules/no-deprecated.md @@ -1,9 +1,5 @@ # import/no-deprecated -**Stage: 0** - -**NOTE**: this rule is currently a work in progress. There may be "breaking" changes: most likely, additional cases that are flagged. - Reports use of a deprecated name, as indicated by a JSDoc block with a `@deprecated` tag or TomDoc `Deprecated: ` comment. diff --git a/src/ExportMap.js b/src/ExportMap.js index c8335d9c8..38aedc6d6 100644 --- a/src/ExportMap.js +++ b/src/ExportMap.js @@ -192,8 +192,6 @@ export default class ExportMap { /** * parse docs from the first node that has leading comments - * @param {...[type]} nodes [description] - * @return {{doc: object}} */ function captureDoc(source, docStyleParsers) { const metadata = {} @@ -202,9 +200,17 @@ function captureDoc(source, docStyleParsers) { // 'some' short-circuits on first 'true' nodes.some(n => { try { + + let leadingComments + // n.leadingComments is legacy `attachComments` behavior - let leadingComments = n.leadingComments || source.getCommentsBefore(n) - if (leadingComments.length === 0) return false + if ('leadingComments' in n) { + leadingComments = n.leadingComments + } else if (n.range) { + leadingComments = source.getCommentsBefore(n) + } + + if (!leadingComments || leadingComments.length === 0) return false for (let name in docStyleParsers) { const doc = docStyleParsers[name](leadingComments) @@ -346,8 +352,6 @@ ExportMap.parse = function (path, content, context) { docStyleParsers[style] = availableDocStyleParsers[style] }) - const source = makeSourceCode(content, ast) - // attempt to collect module doc if (ast.comments) { ast.comments.some(c => { @@ -400,7 +404,7 @@ ExportMap.parse = function (path, content, context) { const existing = m.imports.get(p) if (existing != null) return existing.getter - const getter = () => ExportMap.for(childContext(p, context)) + const getter = thunkFor(p, context) m.imports.set(p, { getter, source: { // capturing actual node reference holds full AST in memory! @@ -411,6 +415,7 @@ ExportMap.parse = function (path, content, context) { return getter } + const source = makeSourceCode(content, ast) ast.body.forEach(function (n) { @@ -496,6 +501,15 @@ ExportMap.parse = function (path, content, context) { return m } +/** + * The creation of this closure is isolated from other scopes + * to avoid over-retention of unrelated variables, which has + * caused memory leaks. See #1266. + */ +function thunkFor(p, context) { + return () => ExportMap.for(childContext(p, context)) +} + /** * Traverse a pattern/identifier node, calling 'callback'