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

Use readFileSync to load package json instead of createRequire #130

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

freaz
Copy link
Member

@freaz freaz commented Feb 13, 2024

Using createRequire, caused problems when OneSDK was bundled for use on Vercel. This way, hopefully, it will properly resolve the URL to file and include it in the bundle.

Needs to be tested, but I don't see better way then publishing it and actually building on Vercel because bulding locally works.

Copy link
Contributor

@TheEdward162 TheEdward162 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay to me, although I'm very surprised readFileSync takes both paths and file URIs while readFile only takes paths.

I've checked the build output in pod and it does look like it's resolving the import to something better now (before it was generating an absolute path at build-time), although I have no idea if it's going to bundle the json file correctly or not because VeRcEl.

@freaz
Copy link
Member Author

freaz commented Feb 19, 2024

I think the most significant difference is not using createRequire because webpack in NextJS isn't properly handling files added this way and aren't included in the final build. My hope, is that using new URL() it will work as we need. But who knows, this still feels like dark magic.

@freaz freaz merged commit 1d087f7 into main Feb 19, 2024
5 checks passed
@freaz freaz deleted the feat/load_package_json_using_fs_read_file branch February 19, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants