Skip to content

Initial wrapper work#1

Merged
fmvilas merged 5 commits intomasterfrom
feature/create-node.js-parser-wrapper
Apr 3, 2019
Merged

Initial wrapper work#1
fmvilas merged 5 commits intomasterfrom
feature/create-node.js-parser-wrapper

Conversation

@rmelian
Copy link
Contributor

@rmelian rmelian commented Mar 23, 2019

What?

This creates an initial Node.js parser. It's a wrapper for the C shared object exported in the Go parser.

Notes

Currently, the binaries for Windows, Mac (darwin), and Linux are included in the bin directory. This makes the package size increase a lot, which is fine for now but we should look for another option in the future. Here are my suggestions:

  • Compile things at npm install time using node-gyp. This is the first approach that I've tried first but didn't succeed. The downside of this approach is that people will need to have Python and Xcode (Mac), gcc in Linux, etc.
  • Build 3 different npm packages for each platform. Potentially more than 3 if we add support for more platforms and archs (x32, x86, etc.)
  • Download the right binary when installing the package (npm install) using a pre-install hook. The downside is that we might have people wanting to use this package in an environment that's not connected to Internet and use their own internal npm registry, like some CI systems.

Food for thought.

@rmelian
Copy link
Contributor Author

rmelian commented Mar 23, 2019

@fmvilas @MikeRalphson do you have an idea how to publish parser C libraries so that we can pull it from the nodejs parser?

What I'm thinking is to publish cparser libraries to a public place, then when installing dependencies for npm create a custom script to download those dependencies to local env.

we could use travis or similar tool to build platform-specific C libraries but then we need a place to store it. maybe Github??

@fmvilas
Copy link
Member

fmvilas commented Mar 23, 2019

To the best of my knowledge, you can put them along with the npm package files. However, that would make the npm package very large and heavy to download, because you'll have all the platform binaries there. I've seen some packages have a compilation process during npm install but honestly I've no idea how this works or if it's even a good practice.

In any case, we could have all the binaries stored somewhere. Like you said, maybe a Github page and a CDN in front of it would be enough.

@fmvilas
Copy link
Member

fmvilas commented Mar 23, 2019

That said, putting all the binaries in the package is not a bad idea as a first step. We can improve this part later.

@fmvilas
Copy link
Member

fmvilas commented Mar 26, 2019

How is this going @rmelian?

@fmvilas fmvilas marked this pull request as ready for review April 3, 2019 12:44
@fmvilas fmvilas changed the title Feature/create node.js parser wrapper Initial wrapper work Apr 3, 2019
@fmvilas fmvilas requested a review from xino12 April 3, 2019 13:19
@fmvilas
Copy link
Member

fmvilas commented Apr 3, 2019

Updated the description of the PR with the following:

What?

This creates an initial Node.js parser. It's a wrapper for the C shared object exported in the Go parser.

Notes

Currently, the binaries for Windows, Mac (darwin), and Linux are included in the bin directory. This makes the package size increase a lot, which is fine for now but we should look for another option in the future. Here are my suggestions:

  • Compile things at npm install time using node-gyp. This is the first approach that I've tried first but didn't succeed. The downside of this approach is that people will need to have Python and Xcode (Mac), gcc in Linux, etc.
  • Build 3 different npm packages for each platform. Potentially more than 3 if we add support for more platforms and archs (x32, x86, etc.)

Food for thought.

@rmelian
Copy link
Contributor Author

rmelian commented Apr 3, 2019

developers will hate us if we make them to do this Compile things at npm install time using node-gyp. This is the first approach that I've tried first but didn't succeed. The downside of this approach is that people will need to have Python and Xcode (Mac), gcc in Linux, etc.

What about the idea of hosting ourselves the binaries and when compiling node we can crate a hook to download the corresponding binaries: I think you told me exatly this once.

@fmvilas
Copy link
Member

fmvilas commented Apr 3, 2019

Yeah, forgot about this option too. We can have a pre-install hook that will download the right binary. Not sure, however, how will it affect people that want to run their CI systems without an Internet connection and using their own internal npm registry.

@fmvilas
Copy link
Member

fmvilas commented Apr 3, 2019

Updated the description with your suggestion. Thanks, @rmelian!

@fmvilas fmvilas merged commit dcfea1b into master Apr 3, 2019
@fmvilas fmvilas deleted the feature/create-node.js-parser-wrapper branch April 3, 2019 14:52
@fmvilas fmvilas restored the feature/create-node.js-parser-wrapper branch June 11, 2019 11:32
@derberg derberg deleted the feature/create-node.js-parser-wrapper branch March 9, 2020 13:46
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

Successfully merging this pull request may close these issues.

2 participants