search

microsoft / node-jsonc-parser

Scanner and parser for JSON with comments.
MIT License
605 stars 53 forks source link

Remove dependency to `vscode-nls` #2

Closed jtheoof closed 7 years ago

jtheoof commented 7 years ago

This is a great parser.

However the fact that it relies on vscode-nls make it less suitable in a browser environment. vscode-nls pulls fs (node filesystem core lib) which is not available in browser. Also it's doing some dynamic require which is not wepback-safe:

{
 "node": {
        "fs": "empty"
    }
}

WARNING in ./~/vscode-nls/lib/main.js 118:23-44 Critical dependency: the request of a dependency is an expression

Without the empty config for fs I would get:

WARNING in ./~/vscode-nls/lib/main.js 118:23-44 Critical dependency: the request of a dependency is an expression

ERROR in ./~/vscode-nls/lib/main.js Module not found: Error: Can't resolve 'fs' in '/Users/jattali/dev/mnubo/front-end/node_modules/vscode-nls/lib' @ ./~/vscode-nls/lib/main.js 7:9-22 @ ./~/jsonc-parser/lib/main.js

Since vscode-nls is only used for localizations of error, I would say it's not critical to remove it.

What do you think @aeschli ?

aeschli commented 7 years ago

good point. I removed the nls dependency with c8c06cc4ea0640f9318487069876f3cd35fa91f4. Released as jsonc-parser@1.0.0

jtheoof commented 7 years ago

Thanks this is great!