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

aws-lambda-nodejs: Does not support Yarn Berry (3.2.x) Pnp (Plug-and-play) #21161

Closed
vleandersson opened this issue Jul 15, 2022 · 5 comments
Closed
Assignees
Labels
@aws-cdk/aws-lambda-nodejs effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2

Comments

@vleandersson
Copy link

vleandersson commented Jul 15, 2022

Describe the bug

Intro

When building using a mono-repo with Yarns Pnp (plug-and-play) mode, AWS CLIs NodejsFunction fails to bundle dependencies. This results in errors like Could not resolve YYY: import XXX from YYY. Using the pure esbuild script, we can easily manage to replicate the issue. But don't be fooled by the reference of an issue in another package - as they got a solution for it.

Diving into the details a bit more

Using a simple monorepo setup by Yarn, we got a parent module (root) with a package.json with only esbuild installed. In the workspace package, we have all our CDK managed resources, as well as a package.json referencing all our build and lambda dependencies.

For yarn without pnp, this works fine, as running eslint in the root folder will still have access to the packages for all child workspaces. Pnp disallows this, leaving esbuild with only access to the root packages.

Esbuilds solution

The solution esbuild came up with is a simple plugin, connecting yarn pnp and eslint. This was born from this discussion

Why this does not work with NodejsFunction

The implementation for NodejsFunction is currently relying on esbuilds CLI. The direction of the community seems to be that the recommended way is to use the Node js API instead, allowing a larger access to esbuild options, including plugins.

That we are not allowed to add plugins to the Esbuild CLI seems to be a design choice that will not change in the near future

Expected Behavior

Current Behavior

yarn berry pnp is not supported

Reproduction Steps

  • Create a new yarn berry monorepo with pnp
  • Install AWS CDK for a child workspace
  • Create a lambda using NodejsFunction
  • Add deps to the child workspace and use these in the referenced lambda
  • Make sure the deps required in the child lambda is not installed in the root package.json
  • run cdk synth

Possible Solution

Suggestion

We change the command we run for esbuild to execute a node command, targeting an esbuild.config.js file. I've drafted a simple implementation that works;

// esbuild.config.js
const { build } = require('esbuild');
const { pnpPlugin } = require('@yarnpkg/esbuild-plugin-pnp');

build({
  entryPoints: [
    'my/entry/point.ts'
  ],
  outfile: 'my.output.file',
  external: [],
  minify: true,
  bundle: true,
  platform: 'node',
  plugins: [pnpPlugin()], // The plugin options are exposed here
  external: [],
  treeShaking: true
}).catch(error => {
  console.error(error);
  process.exit(1);
});

We could then expose the plugins option and add documentation for how to enable pnp with yarn here.

Additional Information/Context

No response

CDK CLI Version

2.31.0

Framework Version

Yarn berry 3

Node.js Version

16.15.0

OS

macOS Monterey

Language

Typescript

Language Version

typescript 4.7.4

Other information

No response

@vleandersson vleandersson added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 15, 2022
@vleandersson vleandersson changed the title aws-lambda-nodejs: Does not support Yarn Berry (3.2.x) Pnp aws-lambda-nodejs: Does not support Yarn Berry (3.2.x) Pnp (Plug-and-play) Jul 15, 2022
@vleandersson
Copy link
Author

vleandersson commented Jul 15, 2022

A workaround that works, but is not great, is to simply turn off the pnp mode in yarn

// .yarnrc.yml
nodeLinker: node-modules

@corymhall
Copy link
Contributor

@vleandersson I saw you commenting on #18175 as well. I didn't realize that one had been closed so I reopened it. There is also another issue that is asking for the library to be re-written to use the esbuild API instead of the CLI.

I am not opposed to updating this module to support those use cases, but the core team will not be able to pick this up to design an implement a solution. I think if the community is able to come up with a design and willing to implement a solution with @jogold help we would be able to provide a final review.

There are a couple of things that concern me with this module and our ability to support it long term.

  1. The nodejs ecosystem changes rapidly. Can we keep up with all the new tools that are released and new project architectures that are used?
  2. I don't think we will ever be able to support every use case that exists. Is there a way to draw some sort of box around what we support in this library? And/or is there a way to make it extensible enough to build 3rd party extensions?

@peterwoodworth peterwoodworth added p2 effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. and removed needs-triage This issue or PR still needs to be triaged. bug This issue is a bug. labels Jul 21, 2022
@shellscape
Copy link

Tossing in another vote for using ESBuild via the API so we can leverage plugins. I quite literally cannot use format: esm with NodeJsFunction because of use of __dirname and the inability to use a plugin to intelligently replace __dirname in the bundle.

@corymhall
Copy link
Contributor

I'm going to close this issue in favor of the other two linked issues.

@github-actions
Copy link

github-actions bot commented Aug 2, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-nodejs effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

4 participants