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

Make PEG.js's Git repository a valid npm pacakge #32

Closed
dmajda opened this issue Aug 13, 2011 · 2 comments
Closed

Make PEG.js's Git repository a valid npm pacakge #32

dmajda opened this issue Aug 13, 2011 · 2 comments
Assignees
Labels
Milestone

Comments

@dmajda
Copy link
Contributor

dmajda commented Aug 13, 2011

PEG.js's Git repository should be a valid npm pacakge, so that development versions can be installed more easily using just the following commands:

git clone git://github.com/dmajda/pegjs.git
npm install ./pegjs

Right now attempt to run these commands ends up with npm error:

npm ERR! Could not install: ./pegjs/
npm ERR! Error: Invalid version: @VERSION
npm ERR! Must be X.Y.Z, with an optional trailing tag.
npm ERR! See the section on 'version' in `npm help json`
npm ERR!     at /usr/local/lib/node_modules/npm/lib/utils/read-json.js:257:13
npm ERR!     at /usr/local/lib/node_modules/npm/lib/utils/read-json.js:132:32
npm ERR!     at P (/usr/local/lib/node_modules/npm/lib/utils/read-json.js:109:40)
npm ERR!     at cb (/usr/local/lib/node_modules/npm/lib/utils/graceful-fs.js:31:9)
npm ERR!     at [object Object].<anonymous> (fs.js:107:5)
npm ERR!     at [object Object].emit (events.js:61:17)
npm ERR!     at afterRead (fs.js:878:12)
npm ERR!     at wrapper (fs.js:245:17)
npm ERR! Report this *entire* log at:
npm ERR!     <http://github.com/isaacs/npm/issues>
npm ERR! or email it to:
npm ERR!     <npm-@googlegroups.com>
npm ERR! 
npm ERR! System Linux 2.6.38-10-generic
npm ERR! command "node" "/usr/local/bin/npm" "install" "./pegjs/"
npm ERR! 
npm ERR! Additional logging details can be found in:
npm ERR!     /home/dmajda/tmp/npm-debug.log
npm not ok

Two things will need to happen to resolve this issue:

  1. The build system needs to avoid concatenating files into one. We should just use require in Node.js environment. For browser environment, the files will still need to be assembled into one, but this is probably doable using Browserify or some similar tool.
  2. The @VERSION substitution should be eliminated at least from the pacakge.json file. This will lead to small duplication of the current version value, but this is lesser evil than making installation of PEG.js development versions easier.
@ghost ghost assigned dmajda Aug 13, 2011
@dmajda
Copy link
Contributor Author

dmajda commented Aug 21, 2011

Apparently Browserify works differently than I thought, so I'd need another tool or write something myself. Moving to post-1.0 milestone, but I'll accept patches for this anytime.

My idea in general is that PEG.js would be a collection of regular Node.js modules (each file in src would become a module) which will be required as needed. Then there would be a script/tool that would process the source code, figure out the dependencies and basically inline them, thus building a browser version. It may also require that the modules have somewhat restricted format (e.g. exactly one assignment to module.exports) because they would probably need to be changed somewhat when inlining.

The tricky point here is avoiding duplication when some module would be required from multiple places. This would be a case with utils.js for example. I still didn't figure out how to do this without introducing boilerplate code.

dmajda added a commit that referenced this issue Nov 10, 2012
When the Git repository will be a npm package, there will be no
preprocessing step and thus no @Version substitution. Let's get rid of
it.

Part of a fix for GH-32.
dmajda added a commit that referenced this issue Nov 10, 2012
PEG.js source code becomes a set of Node.js modules that include each
other as needed. The distribution version is built by bundling these
modules together, wrapping them inside a bit of boilerplate code that
makes |module.exports| and |require| work.

Part of a fix for GH-32.
dmajda added a commit that referenced this issue Nov 10, 2012
Includes:

  * Moving the source code from /src to /lib.
  * Adding an explicit file list to package.json
  * Updating the Makefile.
  * Updating the spec and benchmark suites and their READMEs.

Part of a fix for GH-32.
@dmajda
Copy link
Contributor Author

dmajda commented Nov 10, 2012

This is fixed now, closing.

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

No branches or pull requests

1 participant