Using b.require with react and react-dom results in an error

[casr]

> browserify -r react -r react-dom -p tinyify

/.../node_modules/tinyify/node_modules/common-shakeify/index.js:222
  return row[kDuplicates] || []
            ^

TypeError: Cannot read property 'Symbol(duplicates)' of undefined
    at getDuplicates (/.../node_modules/tinyify/node_modules/common-shakeify/index.js:222:13)
    at analyzer.modules.forEach (/.../node_modules/tinyify/node_modules/common-shakeify/index.js:121:21)
    at Map.forEach (<anonymous>)
    at DestroyableTransform.onend [as _flush] (/.../node_modules/tinyify/node_modules/common-shakeify/index.js:118:22)
    at DestroyableTransform.prefinish (/.../node_modules/readable-stream/lib/_stream_transform.js:138:10)
    at emitNone (events.js:106:13)
    at DestroyableTransform.emit (events.js:208:7)
    at prefinish (/.../node_modules/readable-stream/lib/_stream_writable.js:619:14)
    at finishMaybe (/.../node_modules/readable-stream/lib/_stream_writable.js:627:5)
    at endWritable (/.../node_modules/readable-stream/lib/_stream_writable.js:638:3)

It appears to only happen when I use them both; if I -r them separately then it works fine. I tried it with some simpler packages such as once and xtend together and that appeared to work just fine.

The package.json:

{
  "private": true,
  "scripts": {
    "test": "browserify -r react -r react-dom -p tinyify"
  },
  "dependencies": {
    "react": "16.6.3",
    "react-dom": "16.6.3"
  },
  "devDependencies": {
    "browserify": "16.2.3",
    "tinyify": "2.4.3"
  }
}

[goto-bus-stop]

this might be fixed by a newer common-shakeify version, which this package doesn't yet use as mentioned in browserify/tinyify#14 (comment)

[casr]

I don't believe it is. I have an API example that uses common-shakeify 0.5.2 that results in a similar error:

const commonModules = ['react', 'react-dom']

const g = { global: true }
const commonB = browserify()
commonB.require(commonModules)
commonB.transform('unassertify', g)
commonB.transform(envify({ _: 'purge', NODE_ENV: 'production' }), g)
commonB.transform('uglifyify', g)
commonB.plugin('common-shakeify')
commonB.plugin('browser-pack-flat/plugin')

const commonBundle = commonB.bundle().pipe(require('minify-stream')({ sourceMap: false }))

[goto-bus-stop]

OK, thanks for checking 👍

[goto-bus-stop]

Ah right, it's to do with how row.file and row.id are set when using --require, common-shakeify doesn't pick the right property I think…

it reproduces with just common-shakeify: browserify -r react -r react-dom -p common-shakeify

going to try github's new transfer feature on this issue then!

e; o, it fully removes it from the tinyify issue tracker … i expected/hoped that a sort of "ghost" closed issue in tinyify would remain and link to this. so it's really just for misfiled bugs and not really for things like this. oh well.

[casr]

Might be related with the other work I was doing over at browserify/browserify#1874 which for now I fixed it with more of a workaround.

From what I can gather, I think the underlying issue is that row.id/row.file is pushed through to module-deps to be resolved but row.file does not get updated. I tried a version where in b.require I had a synchronous call to bresolve and set the row.file properly prior to pushing it on to the pipeline. It sorta worked but messed up some of the expose tests (I think).

[casr]

I should add the when using -r/b.require, the top-level row.file is equal to the module name and not its actual file path. Doing some historical search through history showed that this was not always the case.