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.

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

fix: shadow global define in __commonJS #3381

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions internal/bundler_tests/bundler_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1557,3 +1557,29 @@ func TestLoaderBundleWithImportAttributes(t *testing.T) {
`,
})
}

func TestLoaderUmdDefineShouldBeShadowed(t *testing.T) {
loader_suite.expectBundled(t, bundled{
files: map[string]string{
"/entry.js": `
const LZString = "test";
if (typeof define === 'function' && define.amd) {
define(function () { return LZString; });
} else if( typeof module !== 'undefined' && module != null ) {
module.exports = LZString
} else if( typeof angular !== 'undefined' && angular != null ) {
angular.module('LZString', [])
.factory('LZString', function () {
return LZString;
});
}
`,
},
entryPaths: []string{"/entry.js"},
options: config.Options{
Mode: config.ModeConvertFormat,
OutputFormat: config.FormatESModule,
AbsOutputFile: "/out.js",
},
})
}
21 changes: 21 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_loader.txt
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,27 @@ var y_default = "y";
var x_txt = require_x();
console.log(x_txt, y_default);

================================================================================
TestLoaderUmdDefineShouldBeShadowed
---------- /out.js ----------
var require_entry = __commonJS({
"entry.js"(exports, module, define) {
const LZString = "test";
if (typeof define === "function" && define.amd) {
define(function() {
return LZString;
});
} else if (typeof module !== "undefined" && module != null) {
module.exports = LZString;
} else if (typeof angular !== "undefined" && angular != null) {
angular.module("LZString", []).factory("LZString", function() {
return LZString;
});
}
}
});
export default require_entry();

================================================================================
TestRequireCustomExtensionBase64
---------- /out.js ----------
Expand Down
3 changes: 3 additions & 0 deletions internal/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,9 @@ func (g *LinkerGraph) GenerateSymbolImportAndUse(
if ref == repr.AST.ModuleRef {
repr.AST.UsesModuleRef = true
}
if ref == repr.AST.DefineRef {
repr.AST.UsesDefineRef = true
}

// Track that this specific symbol was imported
if sourceIndexToImportFrom != sourceIndex {
Expand Down
2 changes: 2 additions & 0 deletions internal/js_ast/js_ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,7 @@ type AST struct {

ExportsRef ast.Ref
ModuleRef ast.Ref
DefineRef ast.Ref
WrapperRef ast.Ref

ApproximateLineCount int32
Expand All @@ -1545,6 +1546,7 @@ type AST struct {
// here because only the parser checks those.
UsesExportsRef bool
UsesModuleRef bool
UsesDefineRef bool
ExportsKind ExportsKind
}

Expand Down
7 changes: 7 additions & 0 deletions internal/js_parser/js_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ type parser struct {
exportsRef ast.Ref
requireRef ast.Ref
moduleRef ast.Ref
defineRef ast.Ref
importMetaRef ast.Ref
promiseRef ast.Ref
regExpRef ast.Ref
Expand Down Expand Up @@ -14368,6 +14369,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
if p.options.mode == config.ModeBundle && !p.isFileConsideredToHaveESMExports {
p.recordUsage(p.moduleRef)
p.recordUsage(p.exportsRef)
p.recordUsage(p.defineRef)
}

// Mark this scope and all parent scopes as containing a direct eval.
Expand Down Expand Up @@ -17103,10 +17105,12 @@ func (p *parser) prepareForVisitPass() {
// CommonJS-style exports
p.exportsRef = p.declareCommonJSSymbol(ast.SymbolHoisted, "exports")
p.moduleRef = p.declareCommonJSSymbol(ast.SymbolHoisted, "module")
p.defineRef = p.declareCommonJSSymbol(ast.SymbolHoisted, "define")
} else {
// ESM-style exports
p.exportsRef = p.newSymbol(ast.SymbolHoisted, "exports")
p.moduleRef = p.newSymbol(ast.SymbolHoisted, "module")
p.defineRef = p.newSymbol(ast.SymbolHoisted, "define")
}

// Handle "@jsx" and "@jsxFrag" pragmas now that lexing is done
Expand Down Expand Up @@ -17522,6 +17526,7 @@ func (p *parser) toAST(before, parts, after []js_ast.Part, hashbang string, dire
exportsKind := js_ast.ExportsNone
usesExportsRef := p.symbols[p.exportsRef.InnerIndex].UseCountEstimate > 0
usesModuleRef := p.symbols[p.moduleRef.InnerIndex].UseCountEstimate > 0
usesDefineRef := p.symbols[p.defineRef.InnerIndex].UseCountEstimate > 0

if p.esmExportKeyword.Len > 0 || p.esmImportMeta.Len > 0 || p.topLevelAwaitKeyword.Len > 0 {
exportsKind = js_ast.ExportsESM
Expand Down Expand Up @@ -17558,6 +17563,7 @@ func (p *parser) toAST(before, parts, after []js_ast.Part, hashbang string, dire
Symbols: p.symbols,
ExportsRef: p.exportsRef,
ModuleRef: p.moduleRef,
DefineRef: p.defineRef,
WrapperRef: wrapperRef,
Hashbang: hashbang,
Directives: directives,
Expand All @@ -17578,6 +17584,7 @@ func (p *parser) toAST(before, parts, after []js_ast.Part, hashbang string, dire
// CommonJS features
UsesExportsRef: usesExportsRef,
UsesModuleRef: usesModuleRef,
UsesDefineRef: usesDefineRef,
ExportsKind: exportsKind,

// ES6 features
Expand Down
5 changes: 4 additions & 1 deletion internal/linker/linker.go
Original file line number Diff line number Diff line change
Expand Up @@ -4750,8 +4750,11 @@ func (c *linkerContext) generateCodeForFileInChunkJS(
args := []js_ast.Arg{}
if repr.AST.UsesExportsRef || repr.AST.UsesModuleRef {
args = append(args, js_ast.Arg{Binding: js_ast.Binding{Data: &js_ast.BIdentifier{Ref: repr.AST.ExportsRef}}})
if repr.AST.UsesModuleRef {
if repr.AST.UsesModuleRef || repr.AST.UsesDefineRef {
args = append(args, js_ast.Arg{Binding: js_ast.Binding{Data: &js_ast.BIdentifier{Ref: repr.AST.ModuleRef}}})
if repr.AST.UsesDefineRef {
args = append(args, js_ast.Arg{Binding: js_ast.Binding{Data: &js_ast.BIdentifier{Ref: repr.AST.DefineRef}}})
}
}
}

Expand Down