Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hangs when using deasync #4466

Closed
4 tasks done
daveisfera opened this issue Oct 5, 2020 · 14 comments
Closed
4 tasks done

Hangs when using deasync #4466

daveisfera opened this issue Oct 5, 2020 · 14 comments
Labels
invalid not something we need to work on, such as a non-reproducing issue or an external root cause

Comments

@daveisfera
Copy link

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

Using deasync makes the code hang instead of run

Steps to Reproduce

  1. Load test repo: https://github.com/daveisfera/mocha_deasync
  2. Run yarn test and verify it works without mocha
  3. Run yarn mocha and verify that it hangs when running with mocha

Expected behavior: [What you expect to happen]
Runs to competition with mocha as it does without

Actual behavior: [What actually happens]
Running with mocha hangs and prevents us from being able to run our tests

Reproduces how often: [What percentage of the time does it reproduce?]
100%

Versions

  • The output of mocha --version and node node_modules/.bin/mocha --version: 8.1.3
  • The output of node --version: 14.12.0
  • Your operating system
    • name and version: macOS 10.15.6
    • architecture (32 or 64-bit): 64-bit
  • Your shell (e.g., bash, zsh, PowerShell, cmd): zsh 5.7.1 (x86_64-apple-darwin19.0)
  • Your browser and version (if running browser tests): N/A
  • Any third-party Mocha-related modules (and their versions): N/A
  • Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): N/A

Additional Information

We used to use --require to load the settings before the tests, but that doesn't work with native support for ESM in node 14 and esm doesn't yet support optional chaining/nullish coalescing, so loading this "inline" (i.e. doing the deasync in the settings itself upon import) was a solution that worked for running our apps, but didn't work when running our tests in mocha

@juergba
Copy link
Member

juergba commented Oct 6, 2020

I don't trust this esm package. It's not well maintained anymore and I remember having had problems with it in the past.
Could you pass -r esm directly to Node, please? And see what happens?
node -r esm node_modules/mocha/lib/mocha test.js or via NODE_OPTIONS.

@daveisfera
Copy link
Author

Here's the results of the tests with different ways of running with esm:
NODE_OPTIONS="-r esm" node_modules/.bin/mocha test.js - hangs in the same way
node -r esm node_modules/mocha/lib/mocha.js test.js - exits without doing anything
node -r esm node_modules/mocha/bin/mocha test.js - hangs in the same way

Here's a test from running with the ESM support from node 14:
NODE_OPTIONS=--experimental-specifier-resolution=node node_modules/.bin/mocha test.js
And that fails with this error:

yarn run v1.22.10
$ NODE_OPTIONS=--experimental-specifier-resolution=node node_modules/.bin/mocha test.js
internal/process/esm_loader.js:74
    internalBinding('errors').triggerUncaughtException(
                              ^

TypeError [ERR_INVALID_RETURN_PROPERTY_VALUE]: Expected string to be returned for the "format" from the "loader getFormat" function but got type object.
    at Loader.getFormat (internal/modules/esm/loader.js:110:13)
    at async Loader.getModuleJob (internal/modules/esm/loader.js:230:20)
    at async Loader.import (internal/modules/esm/loader.js:164:17)
    at async Object.loadESM (internal/process/esm_loader.js:68:5) {
  code: 'ERR_INVALID_RETURN_PROPERTY_VALUE'
}
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

NOTE: If I added the .js to the import in test.js and remove the need for specifying node resolution through NODE_OPTIONS, then it hangs like the other tests

@daveisfera
Copy link
Author

That error with esm_loader.js is happening with other packages as well, so I opened an issue for that

@juergba
Copy link
Member

juergba commented Oct 6, 2020

node test.mjs
Above doesn't work, without esm, but with extension .mjs?

@juergba
Copy link
Member

juergba commented Oct 6, 2020

We used to use --require to load the settings before the tests, but that doesn't work with native support for ESM in node 14 [...]

Btw I'm quite sure Mocha's require does support ESM modules.

@daveisfera
Copy link
Author

node test.mjs
Above doesn't work, without esm, but with extension .mjs?

Using .mjs hangs as well

We used to use --require to load the settings before the tests, but that doesn't work with native support for ESM in node 14 [...]

Btw I'm quite sure Mocha's require does support ESM modules.

That comment was meant in relation to doing node --require settings.js test.js so that settings would pre-load before the rest of the modules and worked with esm but doesn't work with node's built-in ESM support

@boneskull
Copy link
Member

well, this hangs without mocha by renaming test.js to test.mjs and running node --experimental-specifier-resolution=node test.mjs (node v14). so I'm inclined to think it's not really a mocha-specific issue

@boneskull
Copy link
Member

I really have no idea what is causing readFile to hang, or what deasync is doing

@boneskull
Copy link
Member

you could try --experimental-json-modules and import the .json file, or import { createRequire } from 'module' and const require = createRequire(import.meta.url); require('data.json') but none of this helps you if you need to load some binary blob

@daveisfera
Copy link
Author

Yes, I agree that I could rewrite this example is ways that would work using synchronous file operations, but this is just an example to demonstrate the problem I'm having reading from a database with calls that are only promise based.

Also, I agree that I can do things to node that will reproduce the same sort of behavior (see the linked issues in the original comment), but I was hoping that I could get this to work with esm and mocha because fixing node appears to be a very big change to support pre-loading imports

@boneskull
Copy link
Member

I think what's happening is that Node.js' native module system is taking over, when you want to be using esm instead. I got it to work by doing this:

  1. Rename settings.js to settings.mjs
  2. Rename test.js to test.mjs
  3. Ensure the import statement points to settings.mjs (not settings or settings.js); it must have a file extension
  4. Add {"esm": {"force": true}} to your package.json
  5. rm -rf node_modules/.cache/esm
  6. Run node_modules/.bin/mocha -r esm test.mjs

@boneskull
Copy link
Member

I am not sure what force actually does, but...

@boneskull
Copy link
Member

I'm going to close this, as it's an esm/node issue...

@boneskull boneskull added invalid not something we need to work on, such as a non-reproducing issue or an external root cause and removed unconfirmed-bug labels Oct 8, 2020
@daveisfera
Copy link
Author

daveisfera commented Oct 20, 2020

Just FYI, the update to 8.2.0 makes this happen with node 12 as well (even when using esm and not the built-in ESM support)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid not something we need to work on, such as a non-reproducing issue or an external root cause
Projects
None yet
Development

No branches or pull requests

3 participants