-
Notifications
You must be signed in to change notification settings - Fork 116
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
Proposal - require dependency directly from databases instead of use Store.use
#122
Comments
I could submit pull request if you agree my proposal. |
Unfortunately this module was not designed to be used with webpack... |
Ok. Thanks for the quick reply. |
But why not just require('aws-sdk') from dynamodb.js directly? It can solve this issue. |
because of this: https://github.com/adrai/node-eventstore/blob/master/lib/base.js#L174 |
What about do something like this? dynamodb.js
|
Because I don't want other people like me. having same issue and spend few hours to investigate 😛 |
For sure this will work, but than every implementation needs to do this... that's why we have moved this to a dedicated function. If that is a concern of several people, we can do that. The problem is that all cqrs relevant modules are constructed that way. I also do not understand why it's necessary to use webpack in the backend. In the mean time I would suggest you to use your fork (if you really need webpack) |
Because AWS Lambda doesn't support ES6. And, serverless-webpack is official plugin 😶 |
I know... https://twitter.com/adrirai/status/965700927670931456 |
btw, finally I solved the issue by using babel because babel only do transpile. |
I had a similar problem with |
@adrai Our main reason for using webpack is that we write our code in TypeScript. Having strongly typed code is even more useful in the backend than in the frontend. Basically we have four alternatives now:
What's your opinion? |
@dbartholomae My honest opinion is: I have a "certain" opinion about TypeScript... but this is another story... |
Don't worry, not talking about adding typings in here ;) |
Just my two cents, I don‘t really get the problem to use the library with your favorite transpirier, be it rollup, webpack or parcel. All of them have settings and options which allow this, is the dynamic import the only issue here? |
So far we have multiple days just trying to set this up with webpack and don't see a light at the end of the tunnel yet :-/ |
If you are not set on webpack, I would definitely give parcel a shot. If you supply some repo, I could take a look. The thing is that, I guess your problems come from the dynamic imports, as such i‘m not sure that the proposal from this issue will solve them. |
I just had an idea, which might work for you, if we provide the ability to supply the library with the options so the implementation wont have to require it by itself ( dynamically ), will that work for you? |
We are currently in the works of getting the first prototype live. As soon as we have it (I assume next week) we will share the code base and some ideas for how it could be improved for our usage case :) |
Proposal
require dependency directly from databases instead of use
Store.use
For example:
aws = require('aws-sdk
); instead of aws = Store.use('aws-sdk');I propose this idea because I have some issues.
Background
I using AWS Lambda with serverless and serverless-webpack
I want to use eventstore with dynamodb from lambda but I got some errors. btw, I've installed
aws-sdk
and works on my local.1.
Implementation for db \"dynamodb\" does not exist!
.my code:
then I tried pass the DynamoDB class to eventstore, but I got another error
Cannot find module 'aws-sdk' from parent
my code:
Some of my thoughts
About the issue 1 and 2. I think it's because it cannot find modules via
require
after webpack bundled.https://github.com/adrai/node-eventstore/blob/master/index.js#L42
https://github.com/adrai/node-eventstore/blob/master/lib/databases/dynamodb.js#L2
https://github.com/adrai/node-eventstore/blob/master/lib/base.js#L172
So, that's why I propose this idea.
The text was updated successfully, but these errors were encountered: