unused node.js builtin modules are not tree shaked with define, even unreachable code will break browser build

[shigma]

source file

import { resolve } from 'path'

export function getEntry(name) {
  if (process.env.BROWSER) return require('default')
  return require(resolve(base, name))
}

--bundle --platform=node --tree-shaking --define:process.env.BROWSER=true

I can see from the output that define is working as intended (require(name) get removed) but the "path" module does not get shaked, leaving a unused S=require('path').

Expected behavior: no unused require.

--bundle --platform=browser --tree-shaking --define:process.env.BROWSER=true

The build failed with error: Could not resolve "path".

Expected behavior: build passed.

[shigma]

source file

import { resolve } from 'path'

export function getEntry(name) {
  if (process.env.BROWSER) return require('default')
  // now path is totally unreachable!
  return require(require('path').resolve(base, name))
}

--bundle --platform=node --tree-shaking --define:process.env.BROWSER=true

Everything seems fine.

--bundle --platform=browser --tree-shaking --define:process.env.BROWSER=true

The build still failed with error: Could not resolve "path".

Expected behavior: build passed.

[hyrious]

Basically 2 things:

1. Constant folding currently does not remove unreachable codes like yours, you can write if-else intentionally to make that work:

    export function getEntry(name) {
      if (process.env.BROWSER) return require('default')
      // now path is totally unreachable!
   -  return require(require('path').resolve(base, name))
   +  else return require(require('path').resolve(base, name))
    }
   

2. About the top-level import "path", this is not bundle-able when targetting browser. Choices are using node-polyfills or just mark it as empty module with "browser": { "path": false } in package.json.

[shigma]

@hyrious Thanks for your response.

Constant folding currently does not remove unreachable codes like yours, you can write if-else intentionally to make that work:

So you mean if-else can escape some checks but if-return cannot? Sounds weird to me...