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

feat: add serverless application example #303

Closed
wants to merge 2 commits into from
Closed

feat: add serverless application example #303

wants to merge 2 commits into from

Conversation

vjcspy
Copy link

@vjcspy vjcspy commented Dec 25, 2022

this pr add an example for serverless application. Some points of interest:

  • work offline with serverless-offline
  • bundle with webpack
  • implement jest
  • support dotenv

@NiGhTTraX
Copy link
Owner

Hey @vjcspy, thanks for your PR! Can you please import something from the monorepo in your app, like foo/meaningOfLife and make sure it correctly uses the path aliases?

Also, you can remove the jest and dotenv stuff since I like to keep examples focused only on one thing. jest is already covered in apps/jest.

If you make sure the monorepo imports work, I can take care of cleaning up the rest. Thanks!

@vjcspy
Copy link
Author

vjcspy commented Dec 30, 2022

Thanks for you response. I refactored to make it as simple as possible. Removed jest + dotenv, also used alias path in handler.ts to import app.ts.
it works properly in local with command pnpm -F serverless dev

Copy link
Owner

@NiGhTTraX NiGhTTraX left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason GitHub is not triggering the workflow for this PR, otherwise it would have caught a bunch of lint errors. Please run all of these locally and make sure they pass.

apps/serverless/src/handler.ts Outdated Show resolved Hide resolved
apps/serverless/src/app.ts Outdated Show resolved Hide resolved
apps/serverless/package.json Outdated Show resolved Hide resolved
@NiGhTTraX
Copy link
Owner

Thanks for updating this @vjcspy. I rebased the PR to solve lockfile conflicts and cleaned up a few things (you're going to have to do a force pull). There are still some issues however.

First of all, it looks like serverless-bundle has peer dependencies on React, which seems weird. I don't have any experience with the framework, but I don't see why React would be needed for lambda development.

apps/serverless
└─┬ serverless-bundle
  └─┬ isomorphic-style-loader
    ├── ✕ missing peer react@"^16.8.0 || ^17.0.0"
    └── ✕ missing peer react-dom@"^16.8.0 || ^17.0.0"

Secondly, running the dev script spews quite a lot of warnings when webpack tries to bundle express:

> cross-env NODE_ENV=development sls offline start --stage offline

Bundling with Webpack...
ERROR in ../../../../../../apps/serverless/node_modules/express/lib/application.js 16:19-42
Module not found: Error: Can't resolve 'finalhandler' in '/Users/nightcrawler/github/ts-monorepo/apps/serverless/node_modules/express/lib'

Trying to access the server at localhost:4000 throws another error:

✖ Unhandled exception in handler 'app'.
✖ Error: Cannot find module 'body-parser'
✖ Runtime.ImportModuleError: Error: Cannot find module 'body-parser'

Can you please look into the above? Moreover, I would need some guidance on how to test this locally, and deploy it (to AWS I assume?). If I'm going to merge this example and maintain it, I need to understand what it does and how to work with it.

@vjcspy
Copy link
Author

vjcspy commented Jan 7, 2023

Hi @NiGhTTraX, All of the above issues are caused by pnpm not installing express dependencies. You can check out express's package.json file and see that it relies a lot on other libraries (react, nextjs doesn't). There are 2 options to solve this problem:

  1. Add setting to pnpm install peer dependencies. I prefer this option and think it is necessary. (Nextjs is similar https://github.com/vercel/next.js/blob/canary/.npmrc)
  2. If you still want to keep the settings of pnpm. Instead of using express I can switch back to using http module. It will not require external libraries. (but note that http module is deprecated)

Deploying to AWS is very simple. Follow this document to configure AWS profile, then just run

pnpm -F @nighttrax/serverless deploy

@NiGhTTraX
Copy link
Owner

@vjcspy express doesn't have any peer dependencies. Installing express with pnpm and webpack-ing it works out of the box, so there must be something off with serverless.

Also, what about serverless-bundle's dependency on React?

@vjcspy
Copy link
Author

vjcspy commented Jan 8, 2023

@NiGhTTraX serverless-bundle set webpack resolve.symlinks = false. This is the cause of the error.
You can check it here apps/serverless/node_modules/serverless-bundle/src/webpack.config.js
Currently this package does not allow override webpack config. But try adding this setting for pnpm i see the issue is fixed

node-linker=hoisted

Also, serverless-bundle was used in isomorphic-style-loader for supporting css and scss which depend on react.

I can switch to serverless-webpack. Actually, serverless-bundle uses the serverless-webpack plugin. Then I thought it would be better because we won't include unnecessary things.

@NiGhTTraX
Copy link
Owner

@vjcspy if you can resolve the express issues and remove the React dependency then I'll take another look at this. It MUST work with pnpm's default config.

@vjcspy vjcspy closed this by deleting the head repository Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants