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

require with angular and browserify #339

Closed
pietersv opened this issue Dec 30, 2015 · 4 comments
Closed

require with angular and browserify #339

pietersv opened this issue Dec 30, 2015 · 4 comments

Comments

@pietersv
Copy link
Contributor

Received two issue reports related to the require calls:

The first recommends getting rid of the string concatenations, e.g. jszip = require('js'+'zip');
The seconds adds jszip = require('js'+'zip'); in the case where typeof exports === undefined.

These changes make sense to me. But these relate to core code inherited from the main https://github.com/SheetJS/js-xlsx branch, and go beyond styling which was the intent of the https://github.com/protobi/js-xlsx fork.

  • Q1 Should these changes be made in the main https://github.com/SheetJS/js-xlsx branch?
  • Q2 Why require('js' + 'zip') rather than require('jszip')?
  • Q3 Why not call require('jszip') when exports is undefined?
@tkhyn
Copy link

tkhyn commented Jan 11, 2016

Hello, same concern here. I have errors when importing xslx with jspm's SystemJS browser side.

The issue I have is similar to systemjs/builder#45 (I get Module jszip not declared as a dependency)

This could be solved by replacing the dynamic require('js' + 'zip') by require('jszip'). I second @pietersv's 2nd question!

@csvan
Copy link

csvan commented Dec 7, 2016

+1

@SheetJSDev
Copy link
Contributor

We un-broke all of the require statements in 0.8.7 and included samples with browserify/requirejs/webpack. Please check against the latest version.

@pietersv if you have a very small angularjs project, we can add to the main repo to make sure we don't break compatibility in future versions

@SheetJSDev
Copy link
Contributor

At this point I think we resolved the browserify/angular problems. @tkhyn I am not familiar with SystemJS, but please test against the latest version and raise a new issue if it is still failing.

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

4 participants