public-transport / hafas-client

JavaScript client for HAFAS public transport APIs.
ISC License
269 stars 54 forks source link

support environments without support for createRequire() & JSON modules #300

Open AnsgarH1 opened 11 months ago

AnsgarH1 commented 11 months ago

Hi there, I just tried to deploy a Next.js app with React server components using hafas-client to Vercel. It always worked as intended on my machine, but never when deployed to a lambda function as I always got a "could not import base.json" error. I even went as far as forking the package and replacing the require statement with an import, but that resulted in a lambda timeout. The solution was to use node 16 instead of node 18 in the Vercel config. I also changed the region from Washington D.C. to Frankfurt, but that probably won't make any difference.

I hope this info helped anyone, but it took me like 3 hours to troubleshoot.

Edit: i reverted back to using the official published npm package instead of my fork, but this broke it again and i got the " Cannot find module './base.json'" Error again.

derhuerst commented 11 months ago

I don't think hafas-client needs to change here, so I can't really help here. Thanks for reporting your insights though!

I'm not very familiar with Vercel, but AFAIK this happens because it tries to bundle your hafas-client-code, including hafas-client and other dependencies, into a single file.

possibly related: https://github.com/vercel/ncc/issues/578 possibly related: https://github.com/vercel/ncc/issues/74 possibly related: https://github.com/vercel/ncc/issues/548

derhuerst commented 11 months ago

What do you think about documenting this issue as known at least, e.g. in docs/vercel.md?

mourabitiziyad commented 9 months ago

I exported base.json data as a javascript object file in a fork of this package. This allowed the file (now base.js ) to be properly bundled on vercel. So, I was no longer getting the error. Maybe it wouldn't hurt to change the file format from json to js although we'll need to change all the references to use "import" instead, which I think is worth it as that's a better way of handling file imports.

@derhuerst

derhuerst commented 9 months ago

Maybe it wouldn't hurt to change the file format from json to js although we'll need to change all the references to use "import" instead, which I think is worth it as that's a better way of handling file imports.

When switching to ECMAScript Modules (ESM) in todo, my assumption was that the Import Attributes Proposal, which would formalise JSON imports as they are already experimentally supported by Node.js, will be accepted soon, allowing us solve this without workarounds.

But by now I assume it will still take a while until we can use import attributes, so I'm fine with building a workaround.

I exported base.json data as a javascript object file in a fork of this package. This allowed the file (now base.js ) to be properly bundled on vercel. So, I was no longer getting the error.

You're welcome to submit a PR that modifies the pull-profile-base-data.sh script to generate JS ESM files instead which default-export the JSON.