Skip to content

Commit

Permalink
track and retain intermediate re-exporting files
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Apr 2, 2021
1 parent 62e3bc0 commit cf87028
Show file tree
Hide file tree
Showing 4 changed files with 426 additions and 7 deletions.
234 changes: 234 additions & 0 deletions internal/bundler/bundler_dce_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,240 @@ func TestPackageJsonSideEffectsFalseNoWarningInNodeModulesIssue999(t *testing.T)
})
}

func TestPackageJsonSideEffectsFalseIntermediateFilesUnused(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {foo} from "demo-pkg"
`,
"/Users/user/project/node_modules/demo-pkg/index.js": `
export {foo} from "./foo.js"
throw 'REMOVE THIS'
`,
"/Users/user/project/node_modules/demo-pkg/foo.js": `
export const foo = 123
`,
"/Users/user/project/node_modules/demo-pkg/package.json": `
{ "sideEffects": false }
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestPackageJsonSideEffectsFalseIntermediateFilesUsed(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {foo} from "demo-pkg"
console.log(foo)
`,
"/Users/user/project/node_modules/demo-pkg/index.js": `
export {foo} from "./foo.js"
throw 'keep this'
`,
"/Users/user/project/node_modules/demo-pkg/foo.js": `
export const foo = 123
`,
"/Users/user/project/node_modules/demo-pkg/package.json": `
{ "sideEffects": false }
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestPackageJsonSideEffectsFalseIntermediateFilesChainAll(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {foo} from "a"
console.log(foo)
`,
"/Users/user/project/node_modules/a/index.js": `
export {foo} from "b"
`,
"/Users/user/project/node_modules/a/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/b/index.js": `
export {foo} from "c"
throw 'keep this'
`,
"/Users/user/project/node_modules/b/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/c/index.js": `
export {foo} from "d"
`,
"/Users/user/project/node_modules/c/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/d/index.js": `
export const foo = 123
`,
"/Users/user/project/node_modules/d/package.json": `
{ "sideEffects": false }
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestPackageJsonSideEffectsFalseIntermediateFilesChainOne(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {foo} from "a"
console.log(foo)
`,
"/Users/user/project/node_modules/a/index.js": `
export {foo} from "b"
`,
"/Users/user/project/node_modules/b/index.js": `
export {foo} from "c"
throw 'keep this'
`,
"/Users/user/project/node_modules/b/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/c/index.js": `
export {foo} from "d"
`,
"/Users/user/project/node_modules/d/index.js": `
export const foo = 123
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestPackageJsonSideEffectsFalseIntermediateFilesDiamond(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import {foo} from "a"
console.log(foo)
`,
"/Users/user/project/node_modules/a/index.js": `
export * from "b1"
export * from "b2"
`,
"/Users/user/project/node_modules/b1/index.js": `
export {foo} from "c"
throw 'keep this 1'
`,
"/Users/user/project/node_modules/b1/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/b2/index.js": `
export {foo} from "c"
throw 'keep this 2'
`,
"/Users/user/project/node_modules/b2/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/c/index.js": `
export {foo} from "d"
`,
"/Users/user/project/node_modules/d/index.js": `
export const foo = 123
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestPackageJsonSideEffectsFalseOneFork(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import("a").then(x => assert(x.foo === "foo"))
`,
"/Users/user/project/node_modules/a/index.js": `
export {foo} from "b"
`,
"/Users/user/project/node_modules/b/index.js": `
export {foo, bar} from "c"
export {baz} from "d"
`,
"/Users/user/project/node_modules/b/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/c/index.js": `
export let foo = "foo"
export let bar = "bar"
`,
"/Users/user/project/node_modules/d/index.js": `
export let baz = "baz"
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestPackageJsonSideEffectsFalseAllFork(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
"/Users/user/project/src/entry.js": `
import("a").then(x => assert(x.foo === "foo"))
`,
"/Users/user/project/node_modules/a/index.js": `
export {foo} from "b"
`,
"/Users/user/project/node_modules/b/index.js": `
export {foo, bar} from "c"
export {baz} from "d"
`,
"/Users/user/project/node_modules/b/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/c/index.js": `
export let foo = "foo"
export let bar = "bar"
`,
"/Users/user/project/node_modules/c/package.json": `
{ "sideEffects": false }
`,
"/Users/user/project/node_modules/d/index.js": `
export let baz = "baz"
`,
"/Users/user/project/node_modules/d/package.json": `
{ "sideEffects": false }
`,
},
entryPaths: []string{"/Users/user/project/src/entry.js"},
options: config.Options{
Mode: config.ModeBundle,
AbsOutputFile: "/out.js",
},
})
}

func TestJSONLoaderRemoveUnused(t *testing.T) {
dce_suite.expectBundled(t, bundled{
files: map[string]string{
Expand Down
48 changes: 41 additions & 7 deletions internal/bundler/linker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,14 @@ type jsMeta struct {
}

type importData struct {
// This is an array of intermediate statements that re-exported this symbol
// in a chain before getting to the final symbol. This can be done either with
// "export * from" or "export {} from". If this is done with "export * from"
// then this may not be the result of a single chain but may instead form
// a diamond shape if this same symbol was re-exported multiple times from
// different files.
reExports []nonLocalDependency

sourceIndex uint32
nameLoc logger.Loc // Optional, goes with sourceIndex, ignore if zero
ref js_ast.Ref
Expand Down Expand Up @@ -1600,12 +1608,17 @@ func (c *linkerContext) scanImportsAndExports() {
for _, partIndex := range repr.ast.NamedImports[importRef].LocalPartsWithUses {
partMeta := &repr.meta.partMeta[partIndex]

// Depend on the file containing the imported symbol
for _, resolvedPartIndex := range partsDeclaringSymbol {
partMeta.nonLocalDependencies = append(partMeta.nonLocalDependencies, nonLocalDependency{
sourceIndex: importData.sourceIndex,
partIndex: resolvedPartIndex,
})
}

// Also depend on any files that re-exported this symbol in between the
// file containing the import and the file containing the imported symbol
partMeta.nonLocalDependencies = append(partMeta.nonLocalDependencies, importData.reExports...)
}

// Merge these symbols so they will share the same name
Expand Down Expand Up @@ -1740,6 +1753,7 @@ func (c *linkerContext) createExportsForFile(sourceIndex uint32) {
if importData, ok := c.files[export.sourceIndex].repr.(*reprJS).meta.importsToBind[export.ref]; ok {
export.ref = importData.ref
export.sourceIndex = importData.sourceIndex
nsExportNonLocalDependencies = append(nsExportNonLocalDependencies, importData.reExports...)
}

// Exports of imports need EImportIdentifier in case they need to be re-
Expand Down Expand Up @@ -1892,12 +1906,13 @@ func (c *linkerContext) matchImportsWithExportsForFile(sourceIndex uint32) {
c.cycleDetector = c.cycleDetector[:0]

importRef := js_ast.Ref{OuterIndex: sourceIndex, InnerIndex: uint32(innerIndex)}
result := c.matchImportWithExport(importTracker{sourceIndex: sourceIndex, importRef: importRef})
result, reExports := c.matchImportWithExport(importTracker{sourceIndex: sourceIndex, importRef: importRef}, nil)
switch result.kind {
case matchImportIgnore:

case matchImportNormal:
repr.meta.importsToBind[importRef] = importData{
reExports: reExports,
sourceIndex: result.sourceIndex,
ref: result.ref,
}
Expand All @@ -1910,6 +1925,7 @@ func (c *linkerContext) matchImportsWithExportsForFile(sourceIndex uint32) {

case matchImportNormalAndNamespace:
repr.meta.importsToBind[importRef] = importData{
reExports: reExports,
sourceIndex: result.sourceIndex,
ref: result.ref,
}
Expand Down Expand Up @@ -1998,8 +2014,11 @@ type matchImportResult struct {
ref js_ast.Ref
}

func (c *linkerContext) matchImportWithExport(tracker importTracker) (result matchImportResult) {
func (c *linkerContext) matchImportWithExport(
tracker importTracker, reExportsIn []nonLocalDependency,
) (result matchImportResult, reExports []nonLocalDependency) {
var ambiguousResults []matchImportResult
reExports = reExportsIn

loop:
for {
Expand Down Expand Up @@ -2094,7 +2113,8 @@ loop:
// time, so we emit a warning and rewrite the value to the literal
// "undefined" instead of emitting an error.
symbol.ImportItemStatus = js_ast.ImportItemMissing
c.log.AddRangeWarning(&source, r, fmt.Sprintf("Import %q will always be undefined because there is no matching export", namedImport.Alias))
c.log.AddRangeWarning(&source, r, fmt.Sprintf(
"Import %q will always be undefined because there is no matching export", namedImport.Alias))
} else {
c.log.AddRangeError(&source, r, fmt.Sprintf("No matching export in %q for import %q",
c.files[nextTracker.sourceIndex].source.PrettyPath, namedImport.Alias))
Expand All @@ -2111,10 +2131,15 @@ loop:
for _, ambiguousTracker := range potentiallyAmbiguousExportStarRefs {
// If this is a re-export of another import, follow the import
if _, ok := c.files[ambiguousTracker.sourceIndex].repr.(*reprJS).ast.NamedImports[ambiguousTracker.ref]; ok {
ambiguousResults = append(ambiguousResults, c.matchImportWithExport(importTracker{
// Save and restore the cycle detector to avoid mixing information
oldCycleDetector := c.cycleDetector
ambiguousResult, newReExportFiles := c.matchImportWithExport(importTracker{
sourceIndex: ambiguousTracker.sourceIndex,
importRef: ambiguousTracker.ref,
}))
}, reExports)
c.cycleDetector = oldCycleDetector
ambiguousResults = append(ambiguousResults, ambiguousResult)
reExports = newReExportFiles
} else {
ambiguousResults = append(ambiguousResults, matchImportResult{
kind: matchImportNormal,
Expand All @@ -2137,6 +2162,15 @@ loop:
nameLoc: nextTracker.nameLoc,
}

// Depend on the statement(s) that declared this import symbol in the
// original file
for _, resolvedPartIndex := range c.files[tracker.sourceIndex].repr.(*reprJS).ast.TopLevelSymbolToParts[tracker.importRef] {
reExports = append(reExports, nonLocalDependency{
sourceIndex: tracker.sourceIndex,
partIndex: resolvedPartIndex,
})
}

// If this is a re-export of another import, continue for another
// iteration of the loop to resolve that import as well
if _, ok := c.files[nextTracker.sourceIndex].repr.(*reprJS).ast.NamedImports[nextTracker.importRef]; ok {
Expand All @@ -2163,9 +2197,9 @@ loop:
nameLoc: result.nameLoc,
otherSourceIndex: ambiguousResult.sourceIndex,
otherNameLoc: ambiguousResult.nameLoc,
}
}, nil
}
return matchImportResult{kind: matchImportAmbiguous}
return matchImportResult{kind: matchImportAmbiguous}, nil
}
}

Expand Down
Loading

0 comments on commit cf87028

Please sign in to comment.