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

doesn't import as described #143

Closed
ghost opened this issue Apr 22, 2021 · 9 comments
Closed

doesn't import as described #143

ghost opened this issue Apr 22, 2021 · 9 comments
Labels
bug Something isn't working

Comments

@ghost
Copy link

ghost commented Apr 22, 2021

Bug Report

Describe the bug

importing as described in docs results in the following error:

(node:67008) UnhandledPromiseRejectionWarning: TypeError: yahooFinance.quote is not a function.

Minimal Reproduction

var yahooFinance = require('yahoo-finance2'); yahooFinance.quote('AAPL')

Environment

Browser or Node: node
Node version (if applicable): 14.6.0
Npm version: 6.14.11
Browser verion (if applicable):
Library version (e.g. 1.10.1):

Additional Context

I got it to work with var yahooFinance = require('yahoo-finance2').default but it just feels wrong.

@ghost ghost added the bug Something isn't working label Apr 22, 2021
@gadicc
Copy link
Owner

gadicc commented Apr 22, 2021

Hey @CFautt, thanks for reporting.

Yes, it looks like when require()-ing vs import-ing you need to do this. I guess when #126 lands that won't be necessary anymore.

Can you just let us know where you saw require in the docs? As we should really only be showing examples with import. (If you're using Babel, TypeScript, etc, it's transpiled down to a require statement that checks for modules and automatically returns the .default prop in this case).

When #126 lands I'll make sure to add examples in the README for both and make sure that both forms work well (i.e. no need to ref .default in CommonJS.

Thanks!

@ghost
Copy link
Author

ghost commented Apr 22, 2021

Thanks for responding so quickly @gadicc!
You're right that require is nowhere in the docs, so that's unfair to expect to work. I get the same error when I try importing it like this, though:

import yahooFinance from 'yahoo-finance2'; var test = async function(){ const results = await yahooFinance.quote('AAPL'); console.log(results); }

and need to amend the call of the function to const results = await yahooFinance.default.quote('AAPL'); to make it work.

Thank you for all your work on this project!

@gadicc
Copy link
Owner

gadicc commented Apr 22, 2021

Hey @CFautt, thanks for reporting back, and for your kind words.

That is indeed surprising. What is your setup? Bundlers / transpilers / etc?

I just noticed that in a plain node project, with { "type": "module" } in the package.json, you'll experience this. Is that your use case? If that's the case, #126 (when it lands) will indeed fix this.

@ghost
Copy link
Author

ghost commented Apr 22, 2021

That is exactly my setup, so it seems it will be fixed soon!

@gadicc
Copy link
Owner

gadicc commented May 8, 2021

@CFautt, #126 / #144 is merged to the devel branch. Not released yet but in case you're comfortable with running development versions from github, you could check it out. Otherwise you'll see an automated message here once it's released as a package to npm.

  • Pure/true ESM module that works great with import in a { "type": "module" } project
  • Note in README about the modules (this you can see already)
  • Clear instructions in the above to use require().default for a pure CommonJS project
  • Interop projects (ESM implemented in CJS, by e.g. Babel) work as expected.

@gadicc
Copy link
Owner

gadicc commented Jun 1, 2021

Hey @CFautt, as above, and now released in v1.11.0. Let us know how you go with that. If you're using import, you can drop the .default from your work now (but it's still required for require, as explained in the docs).

@gadicc gadicc closed this as completed Jun 1, 2021
@gadicc gadicc reopened this Jun 1, 2021
@gadicc
Copy link
Owner

gadicc commented Jun 1, 2021

Actually maybe you can report back before we close this issue. Thanks :)

@advaiyalad
Copy link
Contributor

@gadicc I can confirm that this is fixed with the latest version.

@gadicc
Copy link
Owner

gadicc commented Jun 5, 2021

Awesome, @PythonCreator27, thanks for confirming! 🙏

@gadicc gadicc closed this as completed Jun 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants