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

With v8.0.0 geo-tz/all and geo-tz/now do not work #165

Closed
meezaan opened this issue Jan 8, 2024 · 10 comments · Fixed by #166
Closed

With v8.0.0 geo-tz/all and geo-tz/now do not work #165

meezaan opened this issue Jan 8, 2024 · 10 comments · Fixed by #166
Labels

Comments

@meezaan
Copy link

meezaan commented Jan 8, 2024

I'm unable to get either geo-tz/all or geo-tz/now to work in my import statements. Just geo-tz works fine.

When I change my import statement to the following with v8.0.0:

import { find } from 'geo-tz/all';

I get

src/app.service.ts:2:22 - error TS2307: Cannot find module 'geo-tz/all' or its corresponding type declarations.

2 import { find } from 'geo-tz/all';
                       ~~~~~~~~~~~~

[5:52:36 PM] Found 1 error. Watching for file changes.

Removing the /all works fine. /now throws the same error.

Definitely running v8.0.0 as I can see the find-all and find-now .js and .ts files in the dist folder in node_modules.

@evansiroky
Copy link
Owner

I see you're trying this out using TypeScript, but can you share more information about the environment this is occurring in? How is this all being bundled together? Do you have a small example repo that you can share that reproduces this error?

@meezaan
Copy link
Author

meezaan commented Jan 9, 2024

I see you're trying this out using TypeScript, but can you share more information about the environment this is occurring in? How is this all being bundled together? Do you have a small example repo that you can share that reproduces this error?

It's a very simple NestJS application. I'll share a repo soon. Thanks.

@meezaan
Copy link
Author

meezaan commented Jan 13, 2024

@evansiroky Thanks for your patience. I've created https://github.com/meezaan/node-geo-tz-testing.

Clone the repo. Run npm run start:dev and the service should bootstrap just fine in dev mode.

Then in src/app.service.ts if you change import statement on line 2 to import from geo-tz/all you'll see that nest throws the error.

Thank you!

@evansiroky
Copy link
Owner

Thanks for this! I'll take a detailed look after this weekend.

@shengslogar
Copy link

This is due to v8 aliasing paths via package.json exports, and TypeScript not recognizing this out of the box.

TS will resolve this if you set tsconfig.json compilerOptions.module to "Node16". See microsoft/TypeScript#33079 (comment)

@evansiroky
Copy link
Owner

@shengslogar, I thought it had something to do with the package.json configuration. I'm glad you let us know exactly why.

In some testing, I was able to come up with 2 kinda hacky alternatives to make this work out-of-the-box. The first option is to add additional entry points directly to the src directory so that TypeScript will be able to find the appropriate .ts file. So the package.json would look like this:

".": {
  "main": "./dist/find-1970.js",
  "import": "./dist/find-1970.js",
  "require": "./dist/find-1970.js",
  "types": "./dist/find-1970.d.ts"
},
"./all": {
  "main": "./dist/find-all.js",
  "import": "./dist/find-all.js",
  "require": "./dist/find-all.js",
  "types": "./dist/find-all.d.ts"
},
"./now": {
  "main": "./dist/find-now.js",
  "import": "./dist/find-now.js",
  "require": "./dist/find-now.js",
  "types": "./dist/find-now.d.ts"
},
"./src/find-1970": {
  "main": "./dist/find-1970.js",
  "import": "./dist/find-1970.js",
  "require": "./dist/find-1970.js",
  "types": "./dist/find-1970.d.ts"
},
"./src/find-all": {
  "main": "./dist/find-all.js",
  "import": "./dist/find-all.js",
  "require": "./dist/find-all.js",
  "types": "./dist/find-all.d.ts"
},
"./src/find-now": {
  "main": "./dist/find-now.js",
  "import": "./dist/find-now.js",
  "require": "./dist/find-now.js",
  "types": "./dist/find-now.d.ts"
}

The second option would be to create some new directories that match the documented entry point and put the appropriate index.ts files in there, then it will work as expected with the current documentation. However, that doesn't seem ideal since it's copy-pasting code which isn't good for long-term maintenance.

I'd be curious to hear what people think about either alternative or if changing the tsconfig.json in downstream projects would be more appropriate.

@shengslogar
Copy link

@evansiroky I like your first option best. Wondering if it would be best to name the aliases ./dist/... instead of src?

Another option would be to place all files directly in the root directory and avoid package.json exports altogether. I'm not well-versed in Node packages, but some other projects (Lodash, date-fns) do this in their published builds.

If all else fails, I think leaving a README note regarding tsconfig.json would be a valid alternative.

Thanks for making a valuable project!

@evansiroky
Copy link
Owner

🎉 This issue has been resolved in version 8.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@meezaan
Copy link
Author

meezaan commented Jan 25, 2024

Thanks @evansiroky. Busy week so I could not respond but really appreciate it!

@evansiroky
Copy link
Owner

Thanks, @meezaan, I hope this solution works for you. Feel free to reopen if not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants