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

Allow using Parcel 2 for NodejsFunction #7779

Closed
1 of 2 tasks
blimmer opened this issue May 4, 2020 · 7 comments
Closed
1 of 2 tasks

Allow using Parcel 2 for NodejsFunction #7779

blimmer opened this issue May 4, 2020 · 7 comments
Assignees
Labels
@aws-cdk/aws-lambda-nodejs effort/small Small work item – less than a day of effort feature-request A feature should be added or improved.

Comments

@blimmer
Copy link
Contributor

blimmer commented May 4, 2020

Right now, the documentation requires running Parcel^1

```
yarn add parcel-bundler@^1
# or
npm install parcel-bundler@^1
```

There are bug fixes, such as this one (parcel-bundler/parcel#3151 (comment)) that are only made to Parcel v2 and not v1.

Use Case

The aws-xray-sdk-node package relies on async_hooks, and as described in this bug report: parcel-bundler/parcel#3151 , fails to build under Parcel ^1. @gergan actually reported on that issue that he needs a workaround specifically because of the CDK's NodejsFunction reliance on Parcel v1: parcel-bundler/parcel#3151 (comment)

Proposed Solution

Allow using Parcel ^2. I'm not sure why the docs peg to v1 right now - I'm assuming there's a reason that v2 was incompatible for some reason. If it's actually compatible with v2, then we could also update the docs referenced above.

Other

I created a sample repo to show the problem with Parcel here (see the README). I also filed DataDog/datadog-lambda-js#73 with the package that's pulling aws-xray-sdk-node for more context.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@blimmer blimmer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels May 4, 2020
@jogold
Copy link
Contributor

jogold commented May 4, 2020

Hi @blimmer

The reason is that Parcel v2 is still in alpha (https://www.npmjs.com/package/parcel?activeTab=versions, https://github.com/parcel-bundler/parcel/projects/5).

Not totally comfortable to use an alpha version for this construct.

To be updated when Parcel officially releases v2.

@eladb
Copy link
Contributor

eladb commented May 6, 2020

@jogold Could we allow users to fully customize their bundling step once we have docker based bundling?

@eladb eladb added effort/medium Medium work item – several days of effort effort/small Small work item – less than a day of effort and removed effort/medium Medium work item – several days of effort labels May 7, 2020
@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label May 19, 2020
@asterikx
Copy link
Contributor

In response to @eladb, I would prefer co-locating my lambdas in a lambdas folder using Lerna or a Lerna-style setup so that I have full control over bundling and what dependencies are included with each lambda.

.
├── bin
│   └── cdk.ts
├── lambdas
│   ├── node_modules 
│   ├── packages
│   │   ├── lambda-one
│   │   │   ├── node_modules
│   │   │   ├── src
│   │   │   │   └── index.ts
│   │   │   ├── package.json
│   │   │   └── tsconfig.json
│   │   └── lambda-two
│   │       ├── node_modules
│   │       ├── src
│   │       │   └── index.ts
│   │       ├── package.json
│   │       └── tsconfig.json
│   ├── lerna.json
│   ├── package.json
│   └── tsconfig.json
├── lib
│   └── app-stack.ts
├── node_modules 
├── test
│   └── app.test.ts
├── cdk.json
├── jest.config.js
├── package.json
└── tsconfig.json

Parcel would then need to bundle each lambda using the configs (package.json and tsconfig.json) in the respective Lerna package. Is something like this possible with Parcel?

@jogold
Copy link
Contributor

jogold commented Jun 29, 2020

@blimmer NodejsFunction now uses Parcel 2, see #8681

@eladb
Copy link
Contributor

eladb commented Jun 29, 2020

Can we resolve this?

@jogold
Copy link
Contributor

jogold commented Jun 29, 2020

Can we resolve this?

Yes

@blimmer
Copy link
Contributor Author

blimmer commented Jun 29, 2020

Thank you very much! I'm looking forward to the next release to try this out 😄 🎉

@blimmer blimmer closed this as completed Jun 29, 2020
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/small Small work item – less than a day of effort feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

5 participants