dcodeIO / bcrypt.js

Optimized bcrypt in plain JavaScript with zero dependencies.
Other
3.51k stars 267 forks source link

Bower.json has invalid JSON #66

Closed jdgriffith closed 6 years ago

jdgriffith commented 7 years ago
node_modules/bcryptjs/src/bower.json:4
  4:     "version": /*?== VERSION */

When I am running one of my projects through some CI tools, I am getting an error caused by this file. Is this something you guys can fix on your side? If not, I will try do a workaround or ignore. I am specifically using the Flow tool for type validation.

https://github.com/dcodeIO/bcrypt.js/blob/master/src/bower.json#L4

Thanks!

shengt commented 7 years ago

flow check failing because of the error

Ruffio commented 7 years ago

I don't know, but isn't /src/ used to build with? During build the version is changed. If you are not building, can't you use the build bower file in /dist ?

I think you have to write more detailed of what you are doing.

mjomble commented 7 years ago

It causes problems when using Flow to statically analyze the source code. It also parses the sources in node_modules to build type definitions.

Ruffio commented 7 years ago

So how do you suggest that it is fixed? How do other Bower project handle this?

mjomble commented 7 years ago

From a brief test, it seems this could be fixed by renaming src/bower.json so it doesn't have a .json or .js extension (for example, bower.json.template) and updating the build script accordingly.

It's strange that Flow is checking this file at all since none of the sources require it. Seems like Flow processes all .json files it finds in certain directories. Even if I renamed bower.json to blah.json, it triggered the error.

Ruffio commented 7 years ago

bower.json is the correct filename https://bower.io/docs/creating-packages/

mjomble commented 7 years ago

Yes, but it doesn't have to be the name of the template that is used to create the output file.

The current build script reads the contents from src/bower.json (which is not valid JSON and apart from Flow, would probably also fail if used directly with Bower), replaces /*?== VERSION */ with an actual version string and writes the modified contents to bower.json in the root dir.

The output (bower.json in the root dir) is a valid JSON file which gets used by Bower without problems. However, as far as I know, there's no requirement that the input file used by the build script must have the exact same name. In fact, the input contents don't even necessarily need to be in the form of a separate file, it could also just be inlined into the build script as a variable.

The output file should be the only one that needs to conform to Bower's standards.

sla89 commented 6 years ago

It would be great if you can make sure that src/bower.json contains valid json. We are using retirejs and it generates a warning because of src/bower.json (see RetireJS/retire.js#192) Thank you!

Compulsor-zz commented 6 years ago

I worked around this error by changing the value "version" to the value of version which is located in the node_modules/bcryptjs/bower.json file. Flow didn't show any errors at all after this.

0xcaff commented 6 years ago

I worked around this by adding a postinstall script in my package.json and deleting the offending file.

  "scripts": {
    "postinstall": "rm node_modules/protobufjs/src/bower.json"
  },

This script automatically runs after npm install.