search

isaacs / sax-js

A sax style parser for JS
Other
1.09k stars 325 forks source link

Exotic export strategy causes Rollup to fail #224

Closed alex-r-bigelow closed 6 years ago

alex-r-bigelow commented 6 years ago

This may be the wrong place, but after a lot of googling and poking at source code, I think this is where the problem is coming from.

Attempting to use Rollup to bundle a project that depends on xml-js, which in turn depends on sax-js, I get this problem when attempting to create a UMD bundle:

(!) `this` has been rewritten to `undefined`
https://github.com/rollup/rollup/wiki/Troubleshooting#this-is-undefined
node_modules/sax/lib/sax.js
1567:     }())
1568:   }
1569: })(typeof exports === 'undefined' ? this.sax = {} : exports)

The way sax is exported feels a little exotic, and I'm having trouble understanding the purpose of attaching the library to this? Rollup has some settings for setting this contexts, but as this looks like its usage is just part of an export scheme, I don't think that's what they're talking about.

Any clues as to the purpose / context of this?

alex-r-bigelow commented 6 years ago

Nevermind, adding this to rollup.config.js appears to work (for anyone reaching this via google):

  output: { ... },
  moduleContext: {
    'node_modules/sax/lib/sax.js': 'window'
  }

I'll close this for now, but for the developers of this library, it might be worth swapping window for this or something similar

isaacs commented 6 years ago

This is a very old module, and once upon a time, using things both in node and the browser required some exotic tricks.

Patch welcome to remove all that junk and just use standard commonjs style stuff.