kalisio / feathers-s3

Manage S3 objects with FeathersJS
MIT License
14 stars 1 forks source link

Provide a Koa example #9

Open gustojs opened 1 year ago

gustojs commented 1 year ago

All the examples in the readme use Express, but the default for Feathers v5 is Koa. It would be nice to see how to use this library with Koa instead.

Thanks! <3

claustres commented 1 year ago

Hi, I don't think that except the middleware using koa will result in a different setup doesn't it ? But maybe your are thinking about something specifically ? Did you try it with koa and experimented some problems ?

Thanks for your interest and feedback.

Bomberus commented 1 year ago

Some general feedback when I tried to install this library to a feathersjs(v5) projected generated by the CLI.

  1. It does not support typescript, nor does it provide a type declaration. This results in a Typescript error out of the box.
  2. The project is based on expressjs instead of koa (currently recommended by the feathers dev team)
  3. It is quite hard to adjust the barebone example to the boilerplate generated by the CLI.
  4. Maybe a self contained example would be nice, that you can run in a codesandbox / stackblitz container?

Honestly at this point I just quit adjusting my code, removed the dependency and moved on.

claustres commented 1 year ago

Always nice to get some feedback even if we can't probably tackle all of these problems by ourselves.

  1. About typescript I thought it should be possible to use a raw JS project in a type script project otherwise the whole JS ecosystem should be entirely rewritten using type script but maybe I am wrong as I don't still use it. If we at least need a type declaration any help is welcome to build it.

  2. This project is not really "based" on express as @feathersjs/express is only a devDependencies for our tests. It only provides a helpful middleware usable with express but the service is not related to that and should work in any environment.

  3. Could you please detail what problems are you facing exactly so we can improve the docs ?

  4. There is already a self contained example in this folder https://github.com/kalisio/feathers-s3/tree/master/example, you should be able to run it in a container easily. We built it as the most simple possible, as such you should probably easily migrate it to koa as it only uses express to configure feathers REST endpoint.

Bomberus commented 1 year ago

In case you want to follow up this further.

I created a stackblitz container, using the cli with typescript + koa and manually added your library according to the docs:

  1. npm create feathers@latest my-app
  2. npm install @kalisio/feathers-s3
  3. app.use(serveStatic(app.get('node_modules/@kalisio/feathers-s3/lib'))) (instead of express static)
  4. added service (and @ts-ignore because of missing type declaration)
  5. added the index.html like in your example

When running this project (npm run dev) I get the following error:

Error [ERR_REQUIRE_ESM]: require() of ES Module /home/projects/stackblitz-starters-ig2b9r/node_modules/@kalisio/feathers-s3/lib/index.js from /home/projects/stackblitz-starters-ig2b9r/src/app.ts not supported.
Instead change the require of index.js in /home/projects/stackblitz-starters-ig2b9r/src/app.ts to a dynamic import() which is available in all CommonJS modules.

Stackblitz: https://stackblitz.com/edit/stackblitz-starters-ig2b9r?file=src%2Findex.ts Github Repo: https://github.com/Bomberus/feathers-s3-example

claustres commented 1 year ago

The problem is that @kalisio/feathers-s3 is currently delivered as ES module and according to your TS config your are generating a project using commonJS so that require() is not usable on ES modules as stated by the error. It does not seem the TS compiler correctly detect this and switch to import. I tried to switch the TS config to module: 'nodenext' for testing purpose but unfortunately I am not a TS expoert so that it raises others errors, some imports/types probably needs to be adapted to make this work correctly:

src/client.ts:30:20 - error TS2349: This expression is not callable.
  Type 'typeof import("/home/luc/Development/misc/feathers-s3-example/node_modules/@feathersjs/authentication-client/lib/index")' has no call signatures.

30   client.configure(authenticationClient(authenticationOptions))
                      ~~~~~~~~~~~~~~~~~~~~

src/sqlite.ts:14:14 - error TS2349: This expression is not callable.
  Type 'typeof import("/home/luc/Development/misc/feathers-s3-example/node_modules/knex/types/index")' has no call signatures.

14   const db = knex(config!)
                ~~~~
MrAndriy commented 3 days ago

The problem is that @kalisio/feathers-s3 is currently delivered as ES module and according to your TS config your are generating a project using commonJS so that require() is not usable on ES modules as stated by the error. It does not seem the TS compiler correctly detect this and switch to import. I tried to switch the TS config to module: 'nodenext' for testing purpose but unfortunately I am not a TS expoert so that it raises others errors, some imports/types probably needs to be adapted to make this work correctly:

src/client.ts:30:20 - error TS2349: This expression is not callable.
  Type 'typeof import("/home/luc/Development/misc/feathers-s3-example/node_modules/@feathersjs/authentication-client/lib/index")' has no call signatures.

30   client.configure(authenticationClient(authenticationOptions))
                      ~~~~~~~~~~~~~~~~~~~~

src/sqlite.ts:14:14 - error TS2349: This expression is not callable.
  Type 'typeof import("/home/luc/Development/misc/feathers-s3-example/node_modules/knex/types/index")' has no call signatures.

14   const db = knex(config!)
                ~~~~

@gustojs Here short explanation how run it on TS project

tsconfig.json

{
  "ts-node": {
    "files": true
  },
  "compilerOptions": {
    "target": "es2022",
    "module": "NodeNext",
    "moduleResolution": "Node16",
    "outDir": "./lib",
    "rootDir": "./src",
    "declaration": true,
    "strict": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "sourceMap": true,
    "skipLibCheck": true,
  },
  "include": [
    "src"
  ],
  "exclude": [
    "test"
  ]
}

then in service for example storage.ts

import { discard } from 'feathers-hooks-common'
import type { Application, HookContext } from '../../declarations'
import { storageMethods, storagePath } from './storage.shared'
import { NotFound } from '@feathersjs/errors'

export const storage = async (app: Application) => {
  const { Service, getObject } = await import('@kalisio/feathers-s3')
  const config = Object.assign({}, app.get('storage'))
  const service = new Service(config)

  app.use(storagePath, service, {
    // A list of all methods this service exposes externally
    methods: storageMethods,
    // You can add additional custom events to be sent to clients here
    events: []
  })

  app.service(storagePath).path = storagePath

  const getObjectPath = config.getObjectPath || '/storage'
  app.use(getObjectPath + '/*', getObject(service))

  // Initialize hooks
  app.service(storagePath).hooks({
    before: {
      all: [],
      find: [],
      get: [],
      create: [discard('buffer')],
      remove: [discard('buffer')]
    },
    error: {
      get: [
        (hook: HookContext) => {
          if (hook.error.Code === 'NoSuchKey') {
            throw new NotFound('Object not found')
          }
          return hook
        }
      ]
    }
  })
}

// Add this service to the service type index
declare module '../../declarations' {
  interface ServiceTypes {
    [storagePath]: any
  }
}

@claustres I think also would be great add this to documentation.