move const def under main condition - do not init until needed

[wesoledi]

#1154

[wesoledi]

Fix for error, while running 'npm run build' on some enviroments (with restriction on available ports).

The error was:

[webpack-cli] Failed to load '<path>/webpack.config.js' config
[webpack-cli] Error: Timed out waiting for sync-rpc server to start (it should respond with "pong" when sent "ping"):

More here: #1154

[weaverryan]

Can we move ONLY these 2 lines and put the below 4 back on top? For clarity, I'd like to only move the items that are actually creating the problem.

[stof]

even the import of sync-rpc might not be an issue.

[weaverryan]

good point - it is likely just the hasEslintConfiguration that is the problem since it is USING sync-rpc

[wesoledi]

Sync-rpc it is the main problem for this error. It tried to connect on initialization on import so it is mandatory, to fix this issue.
Can we only move this one? Maybe, I didnt tried. Maybe you are right, sync-rpc can stay, but I moved all, because I believe, there is no point to init or import all of this things if we would not use it at all, because of the if condition. Waste of resources in my opinion. Import and use it only when we need to.

[weaverryan]

Waste of resources in my opinion. Import and use it only when we need to.

It is a matter of code clarity. We normally expect require statements to be at the top of the file. If we start requiring them inline, it'll make the code harder to follow. That's a waste of human resources :). That's why we want to move only the one line needed. That one line actually should have always been moved down here because we are USING a library, not just doing a simple require. But the other require are standard and should be at the top.

[stof]

@wesoledi the error in the reported issue has a stack trace pointing to line 13, which means the line calling forceSync(), not the line importing it. So require('sync-rpc') is not the issue.

[stof]

Thus, if we migrate to ESM imports in the future (not planned yet, but it might happen at some point as that's the future of the ecosystem), imports are required to be at the top-level. So our existing convention of requiring deps at the top-level will make that migration easier.

[michalhorak]

Please allow me to ask you to reconsider the initial request.
I just had some disturbing moments looking for a solution in all wrong places at first.

The problem appeared irregularly. When it started to appear, it was always just the first run (bearable). Recently it became much more frequent. As I am sort of learning to use your wonderful software, I first attempted to exclude all possible misconfigurations on my side, removed node_modules, antivirus ... I did not at all know there is any use of eslint (which I do not use with encore) during packing.

After finally reading this thread I manually performed the request in local 'node_modules' and all is well.

IMHO:
Importing files which are not at all used seems as unnecessary delay to me. I was moreover at first confused and thought I have some misconfiguration in .eslintrc.json (which I use only with IDE), and that is somehow parsed by Encore and disturbes it. By various experiments I discovered it is not the case.

In case of transition to ES6 modules you can use import() for the same purpose.
Thank you.

[weaverryan]

Please allow me to ask you to reconsider the initial request.

Oh, I definitely support this PR :). We just got tangled in some final details. I've finished it over in #1175. I'd love if you could confirm that this small change fixes your issue.

Closing this PR - as the original work + new tweaks is over on #1175.

Cheers!