ericf / express-handlebars

A Handlebars view engine for Express which doesn't suck.
BSD 3-Clause "New" or "Revised" License
2.31k stars 383 forks source link

Strip BOM character from templates saved as 'UTF-8 with signature' #77

Closed jimnoble closed 10 years ago

jimnoble commented 10 years ago

By default, my IDE saves text files as 'UTF-8 with signature', which includes a BOM character at the beginning of the file.

Since node's file API doesn't strip this character out (see https://github.com/joyent/node/issues/1918), this ends up leaking out of my template .handlebars file all the way into the HTML response.

This caused me some great confusion since Chrome's debugger doesnt actually show the character, but nevertheless allows it to mess the page layout up.

I can change the encoding to 'UTF-8 without signature', but this is kludgey, and I have to explicitly do it for every handlebars template file I make.

The fix is really simple, one line of code added around line 327 of express-handlebars.js:

fs.readFile(filePath, 'utf8', function (err, file) {
    if (!err) {

        // BEGIN FIX 

        // Remove BOM character if there is one at the start of the file.

        if(file.charCodeAt(0) == 65279) file = file.substr(1);

        // END FIX

        fileCache[filePath] = file;
    }

Can we add it to the official source so I don't need to patch after every update?

ericf commented 10 years ago

Can you open a PR for this? I would also like to think about it more, but on the surface this seems to make sense.

jimnoble commented 10 years ago

New to PR, but I believe this is done. Let me know if I can help further.

rgrove commented 10 years ago

There's no reason to have your IDE write UTF-8 files with a BOM. It serves no useful purpose and creates many headaches. UTF-8 with BOM is such an uncommon and unnecessary format these days that the vast majority of code is not going to expect it or handle it properly.

From The Unicode Standard (version 6.2.0, section 2.6): "Use of a BOM is neither required nor recommended for UTF-8..."

@jimnoble If your IDE makes it impossible to disable writing a BOM by default (which would be pretty surprising to me), the best thing you can do is either file a bug against your IDE or find a better IDE. Even if this PR were accepted, you'll just run into countless more problems with other tools down the road.

ericf commented 10 years ago

@rgrove thanks for providing this info. It now makes sense why I haven't encountered this and heard about this issue form anyone else.

@jimnoble I'm closing this as wontfix based on @rgrove's comments and suggestions.