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: RollupJS reexports detection #59

Merged
merged 2 commits into from
Jul 15, 2021
Merged

fix: RollupJS reexports detection #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46ee883
fix: RollupJS reexports detection
guybedford Jul 12, 2021
2960bc9
ensure js consistency
guybedford Jul 15, 2021
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ EXPORT_STAR_LIB: `Object.keys(` IDENTIFIER$1 `).forEach(function (` IDENTIFIER$2
(`if (` IDENTIFIER$2 `in` EXPORTS_IDENTIFIER `&&` EXPORTS_IDENTIFIER `[` IDENTIFIER$2 `] ===` IDENTIFIER$1 `[` IDENTIFIER$2 `]) return` `;`)?
)?
) |
`if (` IDENTIFIER$2 `!==` ( `'default'` | `"default"` ) (`&& !` (`Object` `.prototype`? `.hasOwnProperty.call(` IDENTIFIER$1 `, ` IDENTIFIER$2 `)` | IDENTIFIER$1 `.hasOwnProperty(` IDENTIFIER$2 `)`))? `)`
`if (` IDENTIFIER$2 `!==` ( `'default'` | `"default"` ) (`&& !` (`Object` `.prototype`? `.hasOwnProperty.call(` IDENTIFIER `, ` IDENTIFIER$2 `)` | IDENTIFIER `.hasOwnProperty(` IDENTIFIER$2 `)`))? `)`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be like this:

Suggested change
`if (` IDENTIFIER$2 `!==` ( `'default'` | `"default"` ) (`&& !` (`Object` `.prototype`? `.hasOwnProperty.call(` IDENTIFIER `, ` IDENTIFIER$2 `)` | IDENTIFIER `.hasOwnProperty(` IDENTIFIER$2 `)`))? `)`
`if (` IDENTIFIER$2 `!==` ( `'default'` | `"default"` ) (`&& !` (`Object` `.prototype`? `.hasOwnProperty.call(` EXPORTS_IDENTIFIER `, ` IDENTIFIER$2 `)` | EXPORTS_IDENTIFIER `.hasOwnProperty(` IDENTIFIER$2 `)`))? `)`

Copy link
Contributor

Choose a reason for hiding this comment

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

Or well, maybe not because EXPORTS_IDENTIFIER also allows module.exports. But we should expect the exact exports identifier, not a generic one.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason this is generic is due to the export names object check for Babel here - https://github.com/guybedford/cjs-module-lexer/blob/master/test/_unit.js#L171.

)
(
EXPORTS_IDENTIFIER `[` IDENTIFIER$2 `] =` IDENTIFIER$1 `[` IDENTIFIER$2 `]` `;`? |
Expand Down
8 changes: 5 additions & 3 deletions lexer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,8 +651,10 @@ function tryParseObjectDefineOrKeys (keys) {
if (ch !== 33/*!*/) break;
pos += 1;
ch = commentWhitespace();
if (source.startsWith(id, pos)) {
pos += id.length;
if (ch === 79/*O*/ && source.startsWith('bject', pos + 1) && source[pos + 6] === '.') {
if (!tryParseObjectHasOwnProperty(it_id)) break;
}
Comment on lines +654 to +656
Copy link
Contributor

Choose a reason for hiding this comment

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

This has just been moved here from the old else branch without changing the semantics, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the order switch is so that pos doesn't get changed for the fallback check.

else if (identifier()) {
ch = commentWhitespace();
if (ch !== 46/*.*/) break;
pos++;
Expand All @@ -669,7 +671,7 @@ function tryParseObjectDefineOrKeys (keys) {
if (ch !== 41/*)*/) break;
pos += 1;
}
else if (!tryParseObjectHasOwnProperty(it_id)) break;
else break;
ch = commentWhitespace();
}
if (ch !== 41/*)*/) break;
Expand Down
Binary file modified lib/lexer.wasm
View file
Open in desktop
Binary file not shown.
170 changes: 88 additions & 82 deletions lib/lexer.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2879,107 +2879,113 @@
i32.const 2
i32.add
i32.store offset=20540
call 39
drop
block ;; label = @6
block ;; label = @7
i32.const 0
i32.load offset=20540
local.tee 0
local.get 2
local.get 6
call 65
br_if 0 (;@7;)
i32.const 0
local.get 0
local.get 7
i32.const 1
i32.shl
i32.add
i32.store offset=20540
call 39
i32.const 46
i32.ne
br_if 6 (;@1;)
i32.const 0
i32.const 0
i32.load offset=20540
i32.const 2
i32.add
i32.store offset=20540
call 39
i32.const 104
local.tee 0
i32.const 79
i32.ne
br_if 6 (;@1;)
br_if 0 (;@7;)
i32.const 0
i32.load offset=20540
local.tee 0
i32.const 2
i32.add
i32.const 97
i32.const 115
i32.const 79
i32.const 119
i32.const 110
i32.const 80
i32.const 114
i32.const 111
i32.const 112
i32.const 98
i32.const 106
i32.const 101
i32.const 114
i32.const 99
i32.const 116
i32.const 121
call 47
i32.const 46
call 38
i32.eqz
br_if 6 (;@1;)
i32.const 0
local.get 0
i32.const 28
i32.add
i32.store offset=20540
call 39
i32.const 40
i32.ne
br_if 6 (;@1;)
i32.const 0
i32.const 0
i32.load offset=20540
i32.const 2
i32.add
i32.store offset=20540
call 39
drop
i32.const 0
i32.load offset=20540
local.tee 0
br_if 0 (;@7;)
local.get 4
local.get 3
call 65
br_if 6 (;@1;)
i32.const 0
local.get 0
local.get 8
i32.const 1
i32.shl
i32.add
i32.store offset=20540
call 39
i32.const 41
i32.ne
call 51
i32.eqz
br_if 6 (;@1;)
i32.const 0
i32.const 0
i32.load offset=20540
i32.const 2
i32.add
i32.store offset=20540
br 1 (;@6;)
end
local.get 0
call 45
i32.eqz
br_if 0 (;@6;)
call 39
i32.const 46
i32.ne
br_if 5 (;@1;)
i32.const 0
i32.const 0
i32.load offset=20540
i32.const 2
i32.add
i32.store offset=20540
call 39
i32.const 104
i32.ne
br_if 5 (;@1;)
i32.const 0
i32.load offset=20540
local.tee 0
i32.const 2
i32.add
i32.const 97
i32.const 115
i32.const 79
i32.const 119
i32.const 110
i32.const 80
i32.const 114
i32.const 111
i32.const 112
i32.const 101
i32.const 114
i32.const 116
i32.const 121
call 47
i32.eqz
br_if 5 (;@1;)
i32.const 0
local.get 0
i32.const 28
i32.add
i32.store offset=20540
call 39
i32.const 40
i32.ne
br_if 5 (;@1;)
i32.const 0
i32.const 0
i32.load offset=20540
i32.const 2
i32.add
i32.store offset=20540
call 39
drop
i32.const 0
i32.load offset=20540
local.tee 0
local.get 4
local.get 3
call 65
br_if 5 (;@1;)
i32.const 0
local.get 0
local.get 8
call 51
i32.eqz
i32.const 1
i32.shl
i32.add
i32.store offset=20540
call 39
i32.const 41
i32.ne
br_if 5 (;@1;)
i32.const 0
i32.const 0
i32.load offset=20540
i32.const 2
i32.add
i32.store offset=20540
end
call 39
local.set 0
Expand Down
7 changes: 4 additions & 3 deletions src/lexer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,8 +641,10 @@ void tryParseObjectDefineOrKeys (bool keys) {
if (ch != '!') break;
pos += 1;
ch = commentWhitespace();
if (memcmp(pos, id_start, id_len * sizeof(uint16_t)) == 0) {
pos += id_len;
if (ch == 'O' && str_eq6(pos + 1, 'b', 'j', 'e', 'c', 't', '.')) {
if (!tryParseObjectHasOwnProperty(it_id_start, it_id_len)) break;
}
else if (identifier(ch)) {
ch = commentWhitespace();
if (ch != '.') break;
pos++;
Expand All @@ -659,7 +661,6 @@ void tryParseObjectDefineOrKeys (bool keys) {
if (ch != ')') break;
pos += 1;
}
else if (!tryParseObjectHasOwnProperty(it_id_start, it_id_len)) break;
ch = commentWhitespace();
}
if (ch != ')') break;
Expand Down
7 changes: 6 additions & 1 deletion test/_unit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,13 @@ suite('Lexer', () => {
if (k !== 'default' && !Object.hasOwnProperty.call(exports, k)) exports[k] = external5[k];
});

const not = require('not');
Object.keys(not).forEach(function (k) {
if (k !== 'default' && !a().hasOwnProperty(k)) exports[k] = not[k];
});

Object.keys(external6).forEach(function (k) {
if (k !== 'default' && !external6.hasOwnProperty(k)) exports[k] = external6[k];
if (k !== 'default' && !exports.hasOwnProperty(k)) exports[k] = external6[k];
});

const external𤭢 = require('external𤭢');
Expand Down