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

Respect package.json type: module for ESM Functions #531

Closed
aaronpowell opened this issue Feb 11, 2022 · 5 comments · Fixed by #552
Closed

Respect package.json type: module for ESM Functions #531

aaronpowell opened this issue Feb 11, 2022 · 5 comments · Fixed by #552
Assignees
Milestone

Comments

@aaronpowell
Copy link

The preview support for ESM (ES6) modules available (#104 for tracking) requires the use of mjs file extensions (https://github.com/Azure/azure-functions-nodejs-worker/blob/v3.x/src/FunctionLoader.ts#L31 for source reference) which is a bit of a pain when it comes to working with TypeScript, you need to use the nightly compiler to output a mjs extension.

It would be good if the worker would respect the type field in package.json and if that is set to module it loads them as ESM rather than commonjs.

@aaronpowell
Copy link
Author

If you're happy for me to PR this in, would you prefer the v3 or v4 branch?

@ejizba
Copy link
Contributor

ejizba commented Feb 11, 2022

v3 branch. That being said, you will need the function app directory from the host in order to read the package.json and I don't think that's currently available. AFAIK, it was first added with the new worker indexing changes, but I would prefer if you let us integrate those changes. You're welcome to investigate a different method for reliably getting the package.json contents (let me know if you find one), but I will likely start that integration work soon anyways.

@aaronpowell
Copy link
Author

My thought was to have the load function walk upwards in the fs to look for the package.json, and if found, read it from there, otherwise fallback to the current logic.

Given that load is called on first invocation, that might have a perf impact, so it could be optimised by caching the first attempt to lookup so future ones don't pay the penalty.

Agreed that the new worker indexing will probably make things simpler, and I wouldn't want to rush that integration, but I'm just exploring how to make it easier to do ESM with TypeScript (in particular), as I keep getting caught on that (so it's totally a self-serving objective here 🤣).

@ejizba
Copy link
Contributor

ejizba commented Feb 17, 2022

We should be conscience that this may surprise some users if suddenly their package.json "type" field is respected, especially since our es6 support is technically in preview. I don't anticipate many people would have their type set to "module" (I don't think it's the default anywhere?) so probably not a big deal.

I think it would be nice if we added support for .cjs files at the same time as we respect "type" per the docs:

Regardless of the value of the "type" field, .mjs files are always treated as ES modules and .cjs files are always treated as CommonJS.

@aaronpowell
Copy link
Author

Agreed, I'd see the decision tree being:

  1. Do what the type field says (module or commonjs)
  2. Check the file extension for .js vs .cjs
  3. Assume commonjs

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

Successfully merging a pull request may close this issue.

3 participants