feat: add pure ESM support to esbuild and default bundlers #1018
Conversation
eduardoboucas
added
the
type: feature
| parseWithEsbuild: false, | ||
| traceWithNft: false, | ||
| zisi_detect_esm: false, |
There was a problem hiding this comment.
This determines whether we'll look at a .js file and figure out whether it's CJS or ESM.
| @@ -84,11 +87,21 @@ export const bundleJsFile = async function ({ | |||
| // URLs, not paths, so even on Windows they should use forward slashes. | |||
| const sourceRoot = targetDirectory.replace(/\\/g, '/') | |||
|
|
|||
| // Configuring the output format of esbuild. The `includedFiles` array we get | |||
There was a problem hiding this comment.
We were not including a package.json when bundling with esbuild. This becomes a problem with ESM functions, since an ESM file and a lack of a package.json with {"type": "module"} would lead to a CJS function with ESM syntax.
(let's get rid of those "as" conversions)
|
added a minor refactoring with TypeScript syntax. was hard to capture in a GitHub suggestion, so committed instead - let me know if you disagree :) 0e95b0f |
| featureFlags: FeatureFlags, | ||
| configVersion?: string, | ||
| ): Promise<{ includedFiles: string[]; moduleFormat: ModuleFormat }> => { | ||
| const packageJsonFile = await getClosestPackageJson(srcDir) |
There was a problem hiding this comment.
I'm not sure why we introduce + use getClosestPackageJson. srcDir would be smth like projectDir/netlify/functions/my-function.ts, right? The difference between getClosesPackageJson and getPackageJson is that one has an optimisation to not read further than the nearest node_modules folder, which would only ever apply if we searched the package.json for a NPM dependency. This isn't the case here - what am I missing?
There was a problem hiding this comment.
getClosestPackageJson returns the contents of the package.json and its path, whereas getPackageJson returns just the contents. I agree that we should consolidate these, but I'd prefer to do that as a separate PR to reduce the amount of changes. What do you think?
There was a problem hiding this comment.
oh, somehow missed this reply. agree! I'll work on that.
I initially had that and was torn on which one was more readable. If you prefer that approach, I'm more than happy to use it! |
Summary
This is a follow-up to #1004, adding support for pure ESM functions to:
.jsextension is found (a468380)