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

Add ESM support for @memlab/api (and remaining) #94

Open
doteric opened this issue Oct 4, 2023 · 4 comments
Open

Add ESM support for @memlab/api (and remaining) #94

doteric opened this issue Oct 4, 2023 · 4 comments

Comments

@doteric
Copy link

doteric commented Oct 4, 2023

Hello,

I would like to request the addition of ESM (instead of just commonjs, preferably both should be supported) support.

Currently for example the __filename is used which is not supported inside ESM, additionally the exports require a full import and then destructing the object instead of only importing what's really needed.

Is there any plan to introduce such support in the near future?

Thank you

➖ ➖ ➖ ➖

➕ Another problem that I have spotted

const dir = path_1.default.join(__dirname, "plugins")

inside TestRunnerLoader

After bundling with esbuild this breaks simply because esbuild builds into one file and looking up files inside a directory like that does not work and anyway in the ESM world such approach is not recommended. I would propose a different option of a list of available plugins and import them natively with js instead of using custom file loading. Generally anything using __dirname or similar should be replaced with a import of some sort 😃

I would highly appreciate a fix for the above 🙇

@doteric doteric changed the title Add ESM support for @memlab/api (and reamining) Add ESM support for @memlab/api (and remaining) Oct 4, 2023
@JacksonGL
Copy link
Member

Thanks for bringing this issues to our attention. At the moment, __filename and __dirname are used in several places in MemLab's code to make it easy to sync MemLab's codebase with Meta's internal codebase. This approach primarily facilitates the synchronization of MemLab's codebase with Meta's internal integration. When a segment of MemLab's code is loaded, the system evaluates the runtime environment to determine the appropriate version of the code to execute (e.g., whether it's for the public release or an internal integration).

To deal with the complications introduced by esbuild's bundling of __filename and __dirname, please consider leveraging the --define option as a potential workaround. The related discussions on this topic can be found here.

However, we recognize the importance of a more robust and forward-looking solution. Before shifting to a more ESM-friendly method by leveraging the native JS import system for plugins, I need to come up with a better strategy for adaptable code synchronization without relying on __filename and __dirname.

@doteric
Copy link
Author

doteric commented Oct 5, 2023

Hey @JacksonGL

Big thanks for the reply 🙇

Is there any specific values you mean the --define to take?
I have tried some workarounds, but it requires several hacky approaches also due to the 2nd issue I have mentioned (the TestRunnerLoader gets plugins based on files in a directory and esbuild does not include them if they are not imported directly). My plan is to simply use memlab as an external part and just include the node_modules of memlab on a different part apart from esbuild unless you have some better ideas 🤔

@JacksonGL
Copy link
Member

@doteric I was suggesting something like this as a short term solution.

esbuild test.js --bundle --format=esm --define:__filename=\"./actual-filename.js\" --outfile=bundle.js

I'm still trying to understand how you use MemLab in your project. If you have a GitHub repo that can replicate the problem that would be great. If that's too much work to create a demo, can you briefly explain how memlab is used in your project? E.g., do you import memlab's API in your node.js JS code and try to use esbuild to make a ESM bundle?

@doteric
Copy link
Author

doteric commented Oct 6, 2023

@JacksonGL
So I am using MemLab inside a AWS Lambda that is compiled using ESBuild. ESBuild uses a flat file structure most of the times unless defined otherwise, so it compiles everything to one single file. Due to that checking the directory for files simply will not work and ESBuild does not know it should compile that code to something else, because it would interfere with the logic. Additionally to that ESM does not support __filename so that's as a bonus to the above problem 😁

Anyway, I have solved both of the cases by using CJS and using a Lambda Layer with @memlab/* and the needed dependencies so that the files are not compiled with ESBuild and left intact which solves the issue.

So the proposition would be to convert the package to fully support ESM, but it might take a bit of of fiddling to make it fully compatible so up to you if you would like to do it some day 😄

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

No branches or pull requests

2 participants