nwbuild reads package.json from ANYWHERE, including dependencies (regression, breaking)

[CosmoMyzrailGorynych]

Issue Type

• [x] Bug
• [ ] Feature
• [ ] Other

Current/Missing Behaviour

I pointed out the issue here: https://github.com/nwutils/nw-builder/pull/600#pullrequestreview-1101841243 And guess what? It found a malformed package.json in some random dependency, breaking the whole pipeline.

Expected/Proposed Behaviour

nwbuild must not use just any package.json. It must use the one placed in the root of the input directory.

Additional Info

• Regression from v3.5.7
• Operating System: any OS
• Node version: v18+
• NW.js version: doesn't matter
• Repro link:
• ...

[ayushmanchhabra]

Since file globing is enabled, we can't assume that the package.json should be in the input directory. Maybe we could have a flag to disable globing on which it looks for package.json in the input directory?

[davedoesdev]

simple-glob supports arrays of globs plus negative matching. Perhaps inserting a negative match for node_modules might work?

[CosmoMyzrailGorynych]

@davedoesdev no. Most apps do need node_modules. And the problem is not just about node_modules folder.

[CosmoMyzrailGorynych]

@ayushmxn my initial idea is to find the deepest common directory from all the input files in the file system, then find package.json as a direct child in it. Something like https://www.npmjs.com/package/common-dir will probably work well here.

[davedoesdev]

Maybe better not to try to guess things and do as @ayushmxn's idea and look for package.json in the input directory, even perhaps with an explicit option.

[CosmoMyzrailGorynych]

@davedoesdev there's no such thing as "input directory" in nw-builder. Only files as an array of glob paths.

https://github.com/nwutils/nw-builder#optionsfiles-required

[davedoesdev]

Yeah, maybe there should be a new option allowing to specify where the package.json lives?

[CosmoMyzrailGorynych]

Yes, it will be a good addition for v4. Yet right now there is a v3.x with a regression (and a glaring hole for options injections) compared to v3.5.7.

[davedoesdev]

@CosmoMyzrailGorynych #600 didn't introduce globbing so I wonder how it could have regressed things? It reduced the number of matches by testing for filename equals to package.json rather than ending in package.json. The list of files tested uses the same globbing as before.

[CosmoMyzrailGorynych]

I never stated that #600 introduce this bug, I used it to highlight a problematic place. The bug was introduced earlier, somewhere between 3.5.7 and 3.8.x. Probably later than 3.7, because these versions did start packing my app. (And failed. Which may or may not happen on 3.8.x, who knows.)

This code started failing after updating:

const nw = new NwBuilder({
    files: ['./app/**', '!./app/export/**', '!./app/projects/**', '!./app/exportDesktop/**', '!./app/cache/**', '!./app/.vscode/**', '!./app/JamGames/**'],
    platforms,
    version: nwVersion,
    downloadUrl: nwSource,
    manifestUrl: nwManifest,
    flavor: 'sdk',
    buildType: 'versioned',
    // forceDownload: true,
    zip: false
});
await nw.build();

My project uses a two-fold structure with outer folder having source files that get outputted in the app directory. It has two main package.json files (for building and for nw.js) and thousands of others in the dependencies.

[ayushmanchhabra]

there's no such thing as "input directory" in nw-builder. Only files as an array of glob paths.

Apologies for the confusion. When I said input directory, I meant the files argument.

Yeah, maybe there should be a new option allowing to specify where the package.json lives?

My initial idea was to have a disableGlob flag which defaults to false.

Yes, it will be a good addition for v4. Yet right now there is a v3.x with a regression (and a glaring hole for options injections) compared to v3.5.7.

Currently trying to address that in #643

My project uses a two-fold structure with outer folder having source files that get outputted in the app directory. It has two main package.json files (for building and for nw.js) and thousands of others in the dependencies.

(Without manually testing) I think this bug was introduced either here or there since they both use globing. I think the solution would be to parse the first instance of package.json and ignore the rest.

I had a look at the ct-js repo. After fixing the above code, one way to make sure the correct package.json is parsed would be to prepend it to the files array. For example:

   files: [ "./path/to/package.json", "./other/**" ]

[ayushmanchhabra]

@CosmoMyzrailGorynych could you please try out v3.8.3-beta.1 and verify if the issue has been fixed?

[CosmoMyzrailGorynych]

I guess? I still can't build the app (wine version problems on my side), but nw-builder did more than before.

I found another regression, though!

const nwSource = void 0;
const nwManifest = void 0;
    var nw = new NwBuilder({
        files: nwFiles,
        platforms,
        version: nwVersion,
     /*   downloadUrl: nwSource,
        manifestUrl: nwManifest,*/
        flavor: 'sdk',
        buildType: 'versioned',
        // forceDownload: true,
        zip: false,
        macIcns: nightly ? './buildAssets/nightly.icns' : './buildAssets/icon.icns'
    });

If you uncomment downloadUrl and manifestUrl, it fails at downloading nw.js binaries:

[22:11:07] 'nwbuild' errored after 1.65 min
[22:11:07] Error: Invalid URI "undefinedv0.67.1/nwjs-sdk-v0.67.1-win-ia32.zip"
    at Request.init (D:\ctjs\node_modules\request\request.js:273:31)
    at new Request (D:\ctjs\node_modules\request\request.js:127:8)
    at request (D:\ctjs\node_modules\request\index.js:53:10)
    at Object.downloadAndUnpack (D:\ctjs\node_modules\nw-builder\lib\downloader.cjs:27:12)
    at D:\ctjs\node_modules\nw-builder\lib\index.cjs:365:20
    at D:\ctjs\node_modules\nw-builder\lib\index.cjs:1000:12
    at D:\ctjs\node_modules\lodash\lodash.js:4967:15
    at baseForOwn (D:\ctjs\node_modules\lodash\lodash.js:3032:24)
    at D:\ctjs\node_modules\lodash\lodash.js:4936:18
    at Function.forEach (D:\ctjs\node_modules\lodash\lodash.js:9410:14)
    at NwBuilder._forEachPlatform (D:\ctjs\node_modules\nw-builder\lib\index.cjs:999:5)
    at NwBuilder.downloadNwjs (D:\ctjs\node_modules\nw-builder\lib\index.cjs:332:8)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async bakePackages (D:\ctjs\gulpfile.js:490:5)

It did work uncommented with void 0 values on v3.5.7.

[ayushmanchhabra]

If you uncomment downloadUrl and manifestUrl, it fails at downloading nw.js binaries

@CosmoMyzrailGorynych Based on the error you have to, try unsetting nwSource and nwManifest. If that doesn't work then try explicitly setting nwSource=https://dl.nwjs.io and nwManifest=https://nwjs.io/versions.json.

[ayushmanchhabra]

I am able to confirm that in v3.8.5 windows and mac builds are not working.

[ayushmanchhabra]

This has been fixed and released in v3.8.6.

[CosmoMyzrailGorynych]

Thanks! I'm back from my business trip and am returning to fiddling with new nw-builder versions.

[ayushmanchhabra]

Let me know if you have any other issues!