require() json files #1357

Closed
isaacs opened this Issue Jul 18, 2011 · 34 comments

Comments

Projects
None yet

isaacs commented Jul 18, 2011

It'd be really handy to be able to require() a JSON file. With this:

require.extensions[".json"] = function (m) {
 m.exports = JSON.parse(fs.readFileSync(m.filename));
};

you could do:

var config = require("./config.json")

dshaw commented Jul 18, 2011

+1

aseemk commented Jul 18, 2011

+500!

I agree. JSON is the de facto standard for configuration or data files in Node.js. I always have to write the same wrapper.

wavded commented Jul 18, 2011

+1

This would be handy, sure. Even though I know it's probably overkill for node I figured I'd throw out the AMD plugins option. It'd also be useful for other kinds of require branching like i18n and other potentially useful resource parsers ("yaml!config.yaml"). Oh, and I think there was some interest in requiring from a zip archive -- all those kind of things could be done easily from userland.

But yeah, another level of misdirection sucks, so my guess is this is a non-starter -- just figured I'd chuck it out there again. I remember there being some interest in getting require.extensions to compose better -- this is one possible approach, but I have no idea how well this would interact with the current require.extensions stuff (my guess is poorly). But one way or another, I'd kill for a straitforward, composable way to hook into require.

+100

+9001

aseemk commented Jul 18, 2011

There needs to be a "Like" or "+1" button for GitHub comments (or whole threads). <3

+1!!!!

Marak commented Jul 18, 2011

FYI, we use nconf for this. Once you get the idea of using json based configuration files, having a friendly interface to your JSON tends to help.

https://github.com/indexzero/nconf

Member

indexzero commented Jul 18, 2011

@isaacs Do you think it would be useful to try to make this operation "safer"?

require.extensions[".json"] = function (m) {
  try {
    m.exports = JSON.parse(fs.readFileSync(m.filename));
  }
  catch (ex) {
    m.exports = {};
  }
};

It seems like it would save a lot of users from having to constantly wrap their require() statements to safe-guard against potential parse errors.

+1 overall though

tj commented Jul 18, 2011

+1 from me, it's small enough to be a "why not"

+1

gf3 commented Jul 18, 2011

@indexzero Good idea, although I'd be more inclined to make it null or false, that way one can differentiate between parse errors and empty files.

Gosh, that's a great idea. I feel really dumb now for doing something similar but not nearly as neat using the vm module.

I had a libYAML binding lying around that I was working on a while ago. Couldn't resist putting this in and publishing to npm (as package libyaml).

The bindings parse a good deal of YAML, and should be able to deal with most anything using it for configuration. Some fancy features (anchors and whatnot) are unimplemented, and serializing is very incomplete.

tj commented Jul 18, 2011

for .json I think you'd want to know of the parse errors, could be potentially confusing otherwise

josh commented Jul 18, 2011

@indexzero Getting a parse error seems consistent with requiring invalid JS. You don't get an empty object back if there is a JS parse error.

Related #584

isaacs commented Jul 18, 2011

@indexzero I think @josh is right. require() is sync, and can throw
if the file isn't found or is invalid, so throwing JSON errors seems
more appropriate than handling them internally.

On Mon, Jul 18, 2011 at 12:49, josh
reply@reply.github.com
wrote:

@indexzero Getting a parse error seems consistent with requiring invalid JS. You don't get an empty object back if there is a JS parse error.

Related #584

Reply to this email directly or view it on GitHub:
joyent#1357 (comment)

isaacs commented Jul 18, 2011

@deanlandolt Yeah, that stuff is a much bigger discussion. I'm of the opinion that stuff like archive require()ing and adding syntax should not be in node-core. JSON parsing is built into the language already, so it's pretty easy.

isaacs closed this Jul 18, 2011

isaacs reopened this Jul 18, 2011

gasi commented Jul 19, 2011

+1

+1

+1

shinuza commented Jul 19, 2011

+42

nice. +1 for not catching errors.

felixge commented Jul 20, 2011

+1

ry commented Jul 20, 2011

+1

isaacs closed this in 588d885 Jul 21, 2011

thurmda commented Jul 21, 2011

I like this idea but I've never felt much pain in loading my configs like this:

var config = require('config')

where config.js is:
module.exports = config = { ... }

Check out this commit:
thurmda/hummingbird@fdb97bb

Technically my config is a module that exports and object literal, which I find a bit nicer than writing strict json ( " vs ' etc.)

A good improvement might be to add a default object when then file is not found.

felixge commented Jul 27, 2011

A good improvement might be to add a default object when then file is not found.

-1, this would bring inconsistencies to the way require works, when a file is not found, it should throw in all cases.

Maybe. I try to avoid exceptions. I would prefer to have null returned or a default config object if set as a second parameter, because that's what I would do next. I'll probably have to wrap require() around my own method.

If you scroll up the thread you'll notice that @josh explained why this is not the best implementation.

armw4 commented Jan 26, 2015

Even all this time later I'd like to just say 👍

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