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

Restructuring to avoid native node extensions #10

Closed
laurence-hudson-tessella opened this issue Jan 25, 2017 · 5 comments
Closed

Restructuring to avoid native node extensions #10

laurence-hudson-tessella opened this issue Jan 25, 2017 · 5 comments

Comments

@laurence-hudson-tessella
Copy link
Contributor

Not an issue, so much as hoping to start a discussion.

I'm currently trying to use xlsx-extract in a context where native node modules just don't seem to be an option (azure's aws lambda knockoff, but the in browser would be another example). As a result the dependency on node-expat is problematic.

I've made changes (see my fork) to replace the node-expat dependency with sax (pure js). It looks like I'm not the first person to try this.

Given that sax is significantly slower than expat it doesn't seem like just merging these changes back to xlsx-extract is good idea, but it would be nice to unify the code bases somewhat.

I think that something along the lines of xlsx-extract-core (BYO xml parser), then xlsx-extract (depends on
xlsx-extract-core, uses node-expat, no name change for backward compatibility) & xlsx-extract-sax (depends on
xlsx-extract-core, uses sax, although the name is already taken on npm), would be a nice place to get to, but might need a little work to set up an adapter pattern.

Is this something you'd be interesting in taking if I do the initial leg work?
@

@laurence-hudson-tessella
Copy link
Contributor Author

@indranath Including your here, as I think you previously looked at replacing expat with sax

@ffalt
Copy link
Owner

ffalt commented Jan 26, 2017

Yes, having the choice would be nice. All forks introducing other than node-expat did so without alternative. My main motivation for xlsx-extract was that it had to handle ridiculous large file sizes and node-expat was one of, if not the only one, which did. It sure would be a relieve for windows users having a hard time compiling node-expat. Please start and I'll see where I can help.

@laurence-hudson-tessella
Copy link
Contributor Author

Yeah, size can be somewhat of a headache. I am successfully getting 300K+ rows out of a 70MB file, but the speed drop going from expat to sax is big.

@ffalt
Copy link
Owner

ffalt commented Jul 3, 2018

node-expat is now an optional package and an option variable, sax-js the default.

@ffalt ffalt closed this as completed Jul 3, 2018
@calebeaires
Copy link

@laurence-hudson-tessella Have you implemented it on browser/client? I have tried to find an definitive solution to import big excel files. This is seens to be the one.

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

No branches or pull requests

3 participants