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

indirect access of 'require' #80

Closed
piotrromanowski opened this issue Apr 29, 2020 · 7 comments
Closed

indirect access of 'require' #80

piotrromanowski opened this issue Apr 29, 2020 · 7 comments

Comments

@piotrromanowski
Copy link

piotrromanowski commented Apr 29, 2020

when trying to use https://github.com/SheetJS/sheetjs
you run into the following error.

node_modules/xlsx/jszip.js:25:135: error: "require" must not be called indirectly

...ts;return (function e(t,n,r){function s(o,u){if(!n[o]){if(!t[o]){var a=typeof require=="function"&&require;if(!u&&a)return a(o,!0);if(i)return i(o,!0);throw new Error("Cannot find module '"+o+"'")}var f=n[...

                                                                                                      ~~~~~~~

Is it intentional? Currently getting around this issue by hosting a copy of the dependency without the typeof require == 'function' calls, but it would be nice if the library allowed those checks

Let me know if there's anything I can do to help! I can possibly dig in and make the exception for this case somewhere here

if e.Ref == p.requireRef && (e != p.callTarget && e != p.typeofTarget) {

@evanw
Copy link
Owner

evanw commented Apr 30, 2020

This is a similar issue to #56, and is intentional.

Bundling with esbuild is intended to traverse your whole dependency tree and include all of your dependencies in the bundle. However, it can only do this if the dependency tree is statically analyzable. This is why esbuild needs every use of require to be a call expression with a string argument. Capturing the value of require and using it indirectly like this is not statically analyzable.

@evanw
Copy link
Owner

evanw commented Apr 30, 2020

One possibility could be to change this into a warning (either by default or as an option). That would allow the build to continue and generate the bundle. The call to require wouldn't work though. It would crash if it was ever used. I believe that would be sufficient to fix this particular library, which was bundled very strangely.

@piotrromanowski
Copy link
Author

Hmm I'm not it's the same case here is it? The code in this file never actually calls require with any arguments that aren't statically analyzable, I think making it a warning would fix the issue.

https://raw.githubusercontent.com/SheetJS/sheetjs/master/jszip.js

Though I agree that this is very strange

@prantlf
Copy link

prantlf commented May 19, 2020

The warning could hide the actual error. Did you try bundling the module delivered by NPM? Instead of bundling UMD modules intended for loading by the browser directly, you can install dependencies like jQuery or JSZip via NPM and let esbuild decide for their most suitable output using their package definition. For example:

npm i jszip

And in your code:

import JSZip from 'jszip'
window.JSZip = JSZip // if some code neds the global variable

This is how I included JSZip in my source module.

@evanw evanw closed this as completed in 792652c Jun 12, 2020
@evanw
Copy link
Owner

evanw commented Jun 12, 2020

This should be fixed in version 0.5.3. It turns out the problem was specifically caused by code bundled with Browserify, so I have special-cased this code pattern in esbuild. You can read more in the release notes.

@atomkirk
Copy link

@prantlf how did you fix this?

 > node_modules/jszip/lib/readable-stream-browser.js:9:25: error: Could not resolve "stream" (set platform to "node" when building for node)
    9 │ module.exports = require("stream");
      ╵                          ~~~~~~~~

1 error

@geoffharcourt
Copy link

geoffharcourt commented Apr 28, 2021

@atomkirk that seems like something that appeared in 0.11.16. You should downgrade to 0.11.15 and it should go away. (I am trying to work up a bug report for it, but don't have enough info yet.)

Here's the issue I opened to discuss or resolve this: #1220

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants