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

Support ESM #233

Closed
grant opened this issue Oct 26, 2020 · 10 comments · Fixed by #292
Closed

Support ESM #233

grant opened this issue Oct 26, 2020 · 10 comments · Fixed by #292

Comments

@grant
Copy link
Contributor

grant commented Oct 26, 2020

Right now this module doesn't support ESM. If you try to use modules, your function will not load.

Provided module can't be loaded.
Is there a syntax error in your code?
Detailed stack trace: Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/timmerman/Documents/trash/esmtestff/index.js
require() of ES modules is not supported.

I believe there are a few things we need to do to allow functions using modules to be supported:

Repro

Allow modules in package.json

 "type": "module",

Use ESM import/export

/**
 * Send "Hello, World!"
 * @param req https://expressjs.com/en/api.html#req
 * @param res https://expressjs.com/en/api.html#res
 */
export const helloWorld = (req, res) => {
  res.send('Hello, World!');
};

Observe error:

Provided module can't be loaded.
Is there a syntax error in your code?
Detailed stack trace: Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: esmtestff/index.js
require() of ES modules is not supported.

Solution

Here's the rough solution I think, after prototyping a bit:

CC: @bcoe

@bcoe
Copy link
Contributor

bcoe commented Oct 27, 2020

@grant it's possible to build a dual-mode module, I just finished doing this for yargs-parser.

This might be worth thinking about for the functions framework.

@grant
Copy link
Contributor Author

grant commented Mar 31, 2021

Alright. So I have a prototype of using dynamic imports in ESM with what I think would work for the FF implementation:

index.js

const USER_FUNCTION = 'myFunc'
const USER_FILE = './foo.js'

async function run() {
  const myFile = await import(USER_FILE);

  const res = myFile[USER_FUNCTION]();
  console.log(res);
}
run();

foo.js

export const myFunc = () => 3;
{
  "name": "dynamic-import",
  "main": "index.js",
  "type": "module",
  "scripts": {
    "start": "node ."
  }
}

You got to be pretty careful with how you load your file.

@mesqueeb
Copy link

@grant isn't "exports" Better than "main" in package.json for esm?

@grant
Copy link
Contributor Author

grant commented Mar 31, 2021

@grant isn't "exports" Better than "main" in package.json for esm?

I don't think the FF user needs to provide module exports like that. Maybe provide an example if you think so?

This case is very different than most modules that support ESM. The FF loads the user's code which can be a ESM, we don't use the FF itself as a module/library (it's a binary).

@mbleigh
Copy link

mbleigh commented May 3, 2021

Hey folks, just wanted to provide a nudge and a backlink to firebase/firebase-tools#2994 -- on the Firebase side we have 50+ developers 👍 requesting ESM support. We can't add it for Firebase until it gets added for FF, so if it's possible to make progress that would be great for our users!

@bcoe
Copy link
Contributor

bcoe commented May 4, 2021

@mbleigh @grant the approach of using dyamic imports seems pretty smart, perhaps we could start with this being something that users opt into, with a slightly modified start script?

Test with the folks who are excited for this feature in firebase, then at some point in the future, make it the default behavior?

@neilstuartcraig
Copy link

Just adding my +1 to this as it's currently preventing me using ES modules in Cloud Functions (not firebase).
I'd be happy to contribute if that's useful.

@koistya
Copy link
Contributor

koistya commented Dec 11, 2021

This feature stopped working today (or, yesterday):

To load an ES module, set "type": "module" in the package.

@arcinston
Copy link

This still isnt working for me any working solution to fix this?

@matthewrobertson
Copy link
Member

@arcinston this feature should be working. Please open a new bug describing your project configuration if you are having issues.

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.

8 participants