Skip to content

Commit

Permalink
fix #1266 by moving closure creation out of parsing scope (#1275)
Browse files Browse the repository at this point in the history
  • Loading branch information
benmosher authored Jan 29, 2019
1 parent 1ec80fa commit e72a336
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 11 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
4 changes: 0 additions & 4 deletions docs/rules/no-deprecated.md
Original file line number Diff line number Diff line change
@@ -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.

Expand Down
28 changes: 21 additions & 7 deletions src/ExportMap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand All @@ -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)
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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!
Expand All @@ -411,6 +415,7 @@ ExportMap.parse = function (path, content, context) {
return getter
}

const source = makeSourceCode(content, ast)

ast.body.forEach(function (n) {

Expand Down Expand Up @@ -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'
Expand Down

0 comments on commit e72a336

Please sign in to comment.