Skip to content

Commit

Permalink
do not emit Object.defineProperty exports (#24)
Browse files Browse the repository at this point in the history
  • Loading branch information
guybedford committed Oct 29, 2020
1 parent 8767c95 commit 89cde88
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 14 deletions.
28 changes: 20 additions & 8 deletions README.md
Expand Up @@ -86,7 +86,7 @@ EXPORTS_SPREAD: `...` COMMENT_SPACE (IDENTIFIER | REQUIRE)
EXPORTS_MEMBER: EXPORTS_DOT_ASSIGN | EXPORTS_LITERAL_COMPUTED_ASSIGN
EXPORTS_DEFINE: `Object` COMMENT_SPACE `.` COMMENT_SPACE `defineProperty COMMENT_SPACE `(` EXPORTS_IDENTIFIER COMMENT_SPACE `,` COMMENT_SPACE IDENTIFIER_STRING
ES_MODULE_DEFINE: `Object` COMMENT_SPACE `.` COMMENT_SPACE `defineProperty COMMENT_SPACE `(` COMMENT_SPACE `__esModule` COMMENT_SPACE `,` COMMENT_SPACE IDENTIFIER_STRING
EXPORTS_LITERAL: MODULE_EXPORTS COMMENT_SPACE `=` COMMENT_SPACE `{` COMMENT_SPACE (EXPORTS_LITERAL_PROP | EXPORTS_SPREAD) COMMENT_SPACE `,` COMMENT_SPACE)+ `}`
Expand All @@ -113,7 +113,9 @@ EXPORT_STAR_LIB: `Object.keys(` IDENTIFIER$1 `).forEach(function (` IDENTIFIER$2
`})`
```

* The returned export names are the matched `IDENTIFIER` and `IDENTIFIER_STRING` slots for all `EXPORTS_MEMBER`, `EXPORTS_DEFINE` and `EXPORTS_LITERAL` matches.
* The returned export names are taken to be the combination of:
1. `IDENTIFIER` and `IDENTIFIER_STRING` slots for all `EXPORTS_MEMBER` and `EXPORTS_LITERAL` matches.
2. `__esModule` if there is an `ES_MODULE_DEFINE` match.
* The reexport specifiers are taken to be the the combination of:
1. The `REQUIRE` matches of the last matched of either `MODULE_EXPORTS_ASSIGN` or `EXPORTS_LITERAL`.
2. All _top-level_ `EXPORT_STAR` `REQUIRE` matches and `EXPORTS_ASSIGN` matches whose `IDENTIFIER` also matches the first `IDENTIFIER` in `EXPORT_STAR_LIB`.
Expand All @@ -125,11 +127,10 @@ EXPORT_STAR_LIB: `Object.keys(` IDENTIFIER$1 `).forEach(function (` IDENTIFIER$2
The basic matching rules for named exports are `exports.name`, `exports['name']` or `Object.defineProperty(exports, 'name', ...)`. This matching is done without scope analysis and regardless of the expression position:

```js
// DETECTS EXPORTS: a, b, c
// DETECTS EXPORTS: a, b
(function (exports) {
exports.a = 'a';
exports['b'] = 'b';
Object.defineProperty(exports, 'c', { value: 'c' });
})(exports);
```

Expand All @@ -141,21 +142,32 @@ Because there is no scope analysis, the above detection may overclassify:
exports.a = 'a';
exports['b'] = 'b';
if (false)
Object.defineProperty(exports, 'c', { value: 'c' });
exports.c = 'c';
})(NOT_EXPORTS, NOT_OBJECT);
```

It will in turn underclassify in cases where the identifiers are renamed:

```js
// DETECTS: NO EXPORTS
(function (e, defineProperty) {
(function (e) {
e.a = 'a';
e['b'] = 'b';
defineProperty(e, 'c', { value: 'c' });
})(exports, defineProperty);
})(exports);
```

#### __esModule Detection

In addition, `__esModule` is detected as an export when set by `Object.defineProperty`:

```js
// DETECTS: __esModule
Object.defineProperty(exports, 'a', { value: 'a' });
Object.defineProperty(exports, '__esModule', { value: true });
```

No other named exports are detected for `defineProperty` calls in order not to trigger getters or non-enumerable properties unnecessarily.

#### Exports Object Assignment

A best-effort is made to detect `module.exports` object assignments, but because this is not a full parser, arbitrary expressions are not handled in the
Expand Down
4 changes: 3 additions & 1 deletion lexer.js
Expand Up @@ -278,7 +278,9 @@ function tryParseObjectDefineOrKeys (keys) {
const exportPos = ++pos;
if (identifier() && source.charCodeAt(pos) === ch) {
// revert for "("
addExport(source.slice(exportPos, pos));
const expt = source.slice(exportPos, pos);
if (expt === '__esModule')
addExport(expt);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/lexer.c
Expand Up @@ -290,7 +290,8 @@ void tryParseObjectDefineOrKeys (bool keys) {
uint16_t* exportPos = ++pos;
if (identifier(*pos) && *pos == ch) {
// revert for "("
addExport(exportPos, pos);
if (pos - exportPos == 10 && str_eq10(exportPos, '_', '_', 'e', 's', 'M', 'o', 'd', 'u', 'l', 'e'))
addExport(exportPos, pos);
}
}
}
Expand Down
6 changes: 2 additions & 4 deletions test/_unit.js
Expand Up @@ -453,10 +453,8 @@ suite('Lexer', () => {
Object.defineProperty(module.exports, 'thing', { value: true });
Object.defineProperty(exports, "__esModule", { value: true });
`);
assert.equal(exports.length, 3);
assert.equal(exports[0], 'namedExport');
assert.equal(exports[1], 'thing');
assert.equal(exports[2], '__esModule');
assert.equal(exports.length, 1);
assert.equal(exports[0], '__esModule');
});

test('module assign', () => {
Expand Down

0 comments on commit 89cde88

Please sign in to comment.