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

Requires the file package to work in node or browserify. #220

Closed
wants to merge 1 commit into from
Closed

Requires the file package to work in node or browserify. #220

wants to merge 1 commit into from

Conversation

jimsynz
Copy link

@jimsynz jimsynz commented Apr 18, 2012

Handlebars depends on the "file" npm package to function under browserify or node.js.

@defunctzombie
Copy link

why does handlebars need this? Could it be removed altogether?

@wagenet
Copy link
Collaborator

wagenet commented May 4, 2012

@shtylman I think this is for precompilation.

@wagenet
Copy link
Collaborator

wagenet commented May 4, 2012

@jamesotron What version of Node? I've installed the Handlebars NPM package and not had any issues.

@defunctzombie
Copy link

I have had the same problem in browserify and script. It works in node tho. Just not when you try to use the client side bundlers.

@jimsynz
Copy link
Author

jimsynz commented May 8, 2012

Sorry for going incommunicado for a few days - my whole family came down with the flu.

Regardless, I'm using Node 0.6.15.

@wagenet
Copy link
Collaborator

wagenet commented May 8, 2012

@jamesotron I have 0.6.11. I wouldn't have expected much significant to change in a minor point release, but maybe it has.

@wagenet
Copy link
Collaborator

wagenet commented May 25, 2012

@jamesotron Will this work for older versions of Node, since we only require >= 0.4.7?

@kpdecker
Copy link
Collaborator

@jamesotron What error are you seeing that led you to this? The only reference to a file module that I could find was in the parser's generated code which as best I can tell is not used.

exports.main = function commonjsMain(args) {
    if (!args[1])
        throw new Error('Usage: '+args[0]+' FILE');
    var source, cwd;
    if (typeof process !== 'undefined') {
        source = require('fs').readFileSync(require('path').resolve(args[1]), "utf8");
    } else {
        source = require("file").path(require("file").cwd()).join(args[1]).read({charset: "utf-8"});
    }
    return exports.parser.parse(source);
}

@jchris
Copy link

jchris commented Dec 29, 2012

I'm getting this when I try to push it to a phonegap environment with browserify and Zepto:

/Users/jchris/code/TouchGap/node_modules/browserify/lib/wrap.js:459
            throw moduleError('Cannot find module');
                  ^
Error: Cannot find module: "file" from directory "/Users/jchris/code/TouchGap/node_modules/handlebars/lib/handlebars/compiler" while processing file /Users/jchris/code/TouchGap/node_modules/handlebars/lib/handlebars/compiler/parser.js
    at moduleError (/Users/jchris/code/TouchGap/node_modules/browserify/lib/wrap.js:420:16)

@kpdecker
Copy link
Collaborator

kpdecker commented Jan 2, 2013

I think this is a result of static analysis via browserify. I have a test branch setup locally that modifies the parser slightly to handle this in a better manner but I need to check into some cleanup there before I can commit it.

@kpdecker kpdecker mentioned this pull request Jan 10, 2013
kpdecker added a commit that referenced this pull request Jan 13, 2013
Removes unnecessary commonjs code generated for the parser. This reduces
the size of the parse by about 700bytes and should resolve lookup issues
with browserify and other static analysis tools. See #220
@kpdecker
Copy link
Collaborator

3cac267 should resolve this issue as the unnecessary code will not be generated anymore.

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

Successfully merging this pull request may close these issues.

5 participants