Skip to content

Commit

Permalink
fix(builtin): strip BOM when parsing package.json (#1453)
Browse files Browse the repository at this point in the history
Fixes #1448
  • Loading branch information
alexeagle authored Dec 14, 2019
1 parent aacd924 commit c65d9b7
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 19 deletions.
13 changes: 7 additions & 6 deletions internal/npm_install/generate_build_file.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,7 @@ package(default_visibility = ["//visibility:public"])
// generate all BUILD files
generateBuildFiles(pkgs);
}
module.exports = {
main,
printPackageBin,
printIndexBzl,
};
exports.main = main;
/**
* Generates all build files
*/
Expand Down Expand Up @@ -468,7 +464,9 @@ def _maybe(repo_rule, name, **kwargs):
function parsePackage(p, hide = true) {
// Parse the package.json file of this package
const packageJson = path.posix.join(p, 'package.json');
const pkg = isFile(packageJson) ? JSON.parse(fs.readFileSync(packageJson, { encoding: 'utf8' })) :
const stripBom = (s) => s.charCodeAt(0) === 0xFEFF ? s.slice(1) : s;
const pkg = isFile(packageJson) ?
JSON.parse(stripBom(fs.readFileSync(packageJson, { encoding: 'utf8' }))) :
{ version: '0.0.0' };
// Trim the leading node_modules from the path and
// assign to _dir for future use
Expand All @@ -492,6 +490,7 @@ def _maybe(repo_rule, name, **kwargs):
hideBazelFiles(pkg);
return pkg;
}
exports.parsePackage = parsePackage;
/**
* Check if a bin entry is a non-empty path
*/
Expand Down Expand Up @@ -958,6 +957,7 @@ nodejs_binary(
}
return result;
}
exports.printPackageBin = printPackageBin;
function printIndexBzl(pkg) {
let result = '';
const executables = _findExecutables(pkg);
Expand Down Expand Up @@ -1001,6 +1001,7 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
}
return result;
}
exports.printIndexBzl = printIndexBzl;
/**
* Given a scope, return the skylark `node_module_library` target for the scope.
*/
Expand Down
20 changes: 8 additions & 12 deletions internal/npm_install/generate_build_file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ function writeFileSync(p: string, content: string) {
/**
* Main entrypoint.
*/
function main() {
export function main() {
// find all packages (including packages in nested node_modules)
const pkgs = findPackages();

Expand All @@ -104,12 +104,6 @@ function main() {
generateBuildFiles(pkgs)
}

module.exports = {
main,
printPackageBin,
printIndexBzl,
};

/**
* Generates all build files
*/
Expand Down Expand Up @@ -515,11 +509,13 @@ function findScopes() {
* package json and return it as an object along with
* some additional internal attributes prefixed with '_'.
*/
function parsePackage(p: string, hide: boolean = true): Dep {
export function parsePackage(p: string, hide: boolean = true): Dep {
// Parse the package.json file of this package
const packageJson = path.posix.join(p, 'package.json');
const pkg = isFile(packageJson) ? JSON.parse(fs.readFileSync(packageJson, {encoding: 'utf8'})) :
{version: '0.0.0'};
const stripBom = (s: string) => s.charCodeAt(0) === 0xFEFF ? s.slice(1) : s;
const pkg = isFile(packageJson) ?
JSON.parse(stripBom(fs.readFileSync(packageJson, {encoding: 'utf8'}))) :
{version: '0.0.0'};

// Trim the leading node_modules from the path and
// assign to _dir for future use
Expand Down Expand Up @@ -1031,7 +1027,7 @@ function additionalAttributes(pkg: Dep, name: string) {
/**
* Given a pkg, return the skylark nodejs_binary targets for the package.
*/
function printPackageBin(pkg: Dep) {
export function printPackageBin(pkg: Dep) {
let result = '';
const executables = _findExecutables(pkg);
if (executables.size) {
Expand All @@ -1058,7 +1054,7 @@ nodejs_binary(
return result;
}

function printIndexBzl(pkg: Dep) {
export function printIndexBzl(pkg: Dep) {
let result = '';
const executables = _findExecutables(pkg);
if (executables.size) {
Expand Down
12 changes: 11 additions & 1 deletion internal/npm_install/test/generate_build_file.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
const fs = require('fs');
const path = require('path');
const {check, files} = require('./check');
const {printPackageBin, printIndexBzl} = require('../generate_build_file');
const {parsePackage, printPackageBin, printIndexBzl} = require('../generate_build_file');

describe('build file generator', () => {
describe('integration test', () => {
Expand All @@ -10,6 +12,14 @@ describe('build file generator', () => {
});
});

describe('parsing package.json', () => {
it('should strip leading Byte-Order Mark character', () => {
const pkgPath = path.join(process.env['TEST_TMPDIR'], 'package.json');
fs.writeFileSync(pkgPath, `\uFEFF{"name": "foo"}`, 'utf-8');
expect(parsePackage(path.dirname(pkgPath)).name).toBe('foo');
});
});

describe('should exclude nodejs_binary rules when', () => {
const pkg = {_name: 'some_name', _dir: 'some_dir', _dependencies: [], _files: []};

Expand Down

0 comments on commit c65d9b7

Please sign in to comment.