eanplatter / forked

:fork_and_knife: forks, forks everywhere.
MIT License
50 stars 4 forks source link

Track json as a dependency and ref it directly #3

Closed agarzola closed 8 years ago

agarzola commented 8 years ago

Fixes #2. Instead of cat package.json | json repository.url do cat package.json | ./node_module/json/lib/json.js repository.url.

eanplatter commented 8 years ago

This path would work for accessing global npm modules, but if I wanted to cd into a node_modules directory in a project it would not be able to find json.

Perhaps if we instead make json a dependency of forked then we can always have direct access to the module, and also be able to control which version forked uses in cases of breaking changes.

Also, I should have written this out in the README somewhere, but the dist/ directory is generated babel code, that is why it looks all wonky :smile:

The src/ directory is what you're looking for. If you update the src/ directory and then run $ npm run dev it will compile the es6 code in src/ to browser ready code in dist/, sorry for the confusion; that's my bad.

EDIT: There is no src/ directory in this project, >.< Just in the index.js for now. That's what I meant when I said src/

eanplatter commented 8 years ago

Actually, it looks like json can only be installed globally, it wouldn't be cool to go installing global node modules on people's computers.

I bet there's a way we can just not use the json library. It's convenient, but seems a bit cumbersome for what we're using it for.

eanplatter commented 8 years ago

@agarzola Here is how I would do it!

In the index.js file

// ...

// remove this old line
// const meta = parser(shell.exec('cat package.json | json repository.url').stdout)

// use shelljs to grab the content of package.json
const packageJson = shell.cat('package.json')

// create a mutable variable for the parsed package
let parsedPackage

// run a try catch block (in case the package.json file doesn't contain a repository key)
try {
  // define the mutable parsedPackage variable as a parsed version of the package.json
  parsedPackage = JSON.parse(packageJson).repository.url
} catch (e) {
  // or throw a new error explaining that we couldn't find a repo
  throw new Error('I could not find a GitHub repository for this module. Please check the modules package.json')
}

// assign the meta variable to the parser method parsing the parsedPackage :P
const meta = parser(parsedPackage)

// ...

Wham! no extra deps or reliance on global npm modules.

Give it a shot! Especially if you're new to babel, it's a cool way to learn :smiley: I'm gonna try and get some sleep, but please let me know if you have questions and I'll respond as soon as I am conscious.

agarzola commented 8 years ago

This uses JSON.parse() as you suggested, and also does a bit more error handling than you suggested so that end users get a clearer picture of what’s wrong (i.e. distinguish between missing package.json file and missing repository url).

Incidentally, I also learned package is a reserved keyword in JavaScript. Learn something new every day.

eanplatter commented 8 years ago

Nice, I'll test this out, and yes, I too learned that package was a reserved word last night haha.

eanplatter commented 8 years ago

Neat! Looks good to me! Thanks man!

agarzola commented 8 years ago

Thank you for letting me help!