Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

Detect functions with ES Modules #617

Closed
eduardoboucas opened this issue Aug 20, 2021 · 5 comments · Fixed by #625
Closed

Detect functions with ES Modules #617

eduardoboucas opened this issue Aug 20, 2021 · 5 comments · Fixed by #625
Assignees
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality

Comments

@eduardoboucas
Copy link
Member

Currently, functions with ES Modules are supported only when using esbuild. It would be great if zip-it-and-ship-it could detect when a function uses ES Modules and automatically use esbuild to bundle it, just like what happens with TypeScript.

@eduardoboucas eduardoboucas added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Aug 20, 2021
@charliegroll
Copy link
Member

POC for possible module detection: #625

@eduardoboucas
Copy link
Member Author

(Adding some loose thoughts here.)

My main concern with this feature is the performance implications of doing an additional parse of each function just to determine whether it uses ESM or not. When using the legacy bundler, we're already parsing files with acorn (via precinct), but I don't think we can tap into that data.

This feels particularly wasteful when processing functions generated dynamically by build plugins (e.g. Essential Next.js). Plugin authors (i.e. Netlify, in the case of Essential plugins) are in control of the format used in these functions, so it feels redundant to be detecting it by performing an expensive operation. If plugins want to generate JavaScript functions that use ESM, they can simply generate .mjs files (once #619 lands).

These functions are loaded from an internal functions directory (i.e. .netlify/functions-internal), which is one of the sources supplied to zipFunctions. We could look into doing the automatic detection only for user-defined functions.

@ehmicky
Copy link
Contributor

ehmicky commented Aug 24, 2021

💯 on what @eduardoboucas is saying about the performance cost of parsing or lexing JavaScript for every build with functions.

I am wondering if a solution based on string search might be good enough for 99% of cases (while still imperfect)? Something like:

  • Read file (or only read first few lines if we are concerned about the time to read big files)
  • Look for the "\nimport " or "\nexport " strings (or "import "/"export " at the beginning of the file) using a string search instead of RegExps (for performance).

This does feel rough and improper compared to using a lexer (even if implemented in WebAssembly). However, it might be much faster. Also, we want to switch to esbuild for all builds anyway in the future, so this solution would eventually be temporary.

@charliegroll
Copy link
Member

@eduardoboucas @ehmicky perf is the concern I had in mind when I introduced https://github.com/guybedford/es-module-lexer in my draft PR, since it runs inline wasm and claims to be highly performant at just doing its one task. If adding any checks is still a concern, though, I can shelf my POC work.

@eduardoboucas
Copy link
Member Author

Thanks for that additional context, @charliegroll. If that library brings considerable performance improvements over acorn, I'm actually interested in finding out whether we could use it instead of acorn for the esbuild parsing.

Let's not shelve your PoC! I think we should continue exploring our options here. This is a sensitive change because it touches the happy path of the vast majority of functions users, so we need to tread carefully (for example, we would want to put this behind a feature flag).

It'd be good to get the PR in a mergeable state (I think some tests are failing at the moment) and then run some benchmarks to have a sense of the impact this change would have on projects of different sizes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Development

Successfully merging a pull request may close this issue.

3 participants