components icon indicating copy to clipboard operation
components copied to clipboard

Provide non conflicting way to work with locally developed components

Open medikoo opened this issue 6 years ago • 5 comments

As implemented currently, components when loaded are not resolved via Node.js resolution rules. Instead they're installed from npm registry into ~/.serverless/components/registry/npm, and loaded from that location.

This doesn't provide a reliable way to work with components which we currently develop, and introduces a dependency on given component being already published to npm registry (problem already reported at https://github.com/serverless/template/issues/6)

Possible workaround is to replace references with local paths directly in serverless.yml (e.g. replace component: my-component with component: ./path/to/my-component), still that messes with git status, and applying such changes back and forth manually is quite inconvenient.

It's additionally problematic if component we develop is a deep dependency of a component we use.

I think it'll be great to have an option (e.g. via env var) to resolve components naturally via Node.js resolution rules, as then such component dependencies can be just linked (and all popular package managers have dedicated commands that setup that: e.g. npm link, yarn link). Such setup also wouldn't interfere with actual code-base of a project

medikoo avatar Oct 29 '19 12:10 medikoo

Other related issue, is that when debugging things, each component uses it's own @serverless/core instance, so introducing any breakpoints in that package (or any other shared by components) won't work as expected.

Also using this.load(..) instead of require raises implication on its own. Connections between components are not visible to static analysis tools that resolve CJS module trees (e.g. ones that could be used to bundle only used modules for provider deployments).

What might be worth to consider, is to make this.load(...) as the method to load remote components (from components registry) only, and for local loads promote regular require usage. That I guess will make handling of it easier

medikoo avatar Oct 29 '19 16:10 medikoo

I'm not able to even use local references. When I try to deploy application with locally developed (forked) serverless next.js component, with following serverless.yml:

myapp:
  component: "../serverless-next.js/packages/serverless-nextjs-component"

Then I get following error when using sls deploy:

sls deploy

  error:
  Error: "component" input is required to run custom methods
    at Proxy.<anonymous> (/usr/local/lib/node_modules/serverless/node_modules/@serverless/template/utils.js:350:13)
    at Object.runComponents (/usr/local/lib/node_modules/serverless/node_modules/@serverless/cli/src/index.js:209:40)

Currently it's simply not possible to develop serverless components locally.

Ideally would would link local component with npm link serverless-next.js and serverless would use it instead.

sheerun avatar Nov 07 '19 13:11 sheerun

I've somewhat hacked it in node-modules-resolution branch, and that works for me - still it's by no means a final solution to consider.

It requires a bit more thought. Finding one reliable way to load components, which with certain env settings will result in loading a remotely hosted component, and with other env setting will result in loading a locally installed one, is challenging.

medikoo avatar Nov 07 '19 16:11 medikoo

@medikoo if it is of any use to you, here is an example how we did it for Webiny. We've creted our own CLI, and our own template component to bypass the component download, and we only resolve component paths from your project using require.resolve: https://github.com/webiny/webiny-js/blob/master/packages/cli/sls/template/utils.js#L151

Our project is also a monorepo, and the default component resolution was a major issue so we made our own Template component which isa based on the original one, but includes many tweaks to fit our needs.

Regarding this.load - I'm also not too happy it is done that way, but internally this.load builds a state file name to include all the parents (so it is recursive). Using import would introduce a problem of not knowing the context of execution: who's your parent? what state file to load?

If you need more info let me know. Cheers!

Pavel910 avatar Nov 28 '19 13:11 Pavel910

Thanks @Pavel910 for sharing that. I've just learned that require.resolve supports options.paths and it's supported since Node.js v8. That's great to know :)

Using import would introduce a problem of not knowing the context of execution: who's your parent?

Indeed, still that could be solved by passing parent to component constructor (e.g. instead of await this.load('component'), it could be require('component').loadIn(this)).

But technically load is also meant to invoke remote component instances which are deployed to the cloud, and in that case it's hard to make it in any other way then via await this.load('component').

Maybe solution is keep this.load, but also provide alternative way for loading locally installed components via something as Component.loadIn(parentInstance)

medikoo avatar Nov 29 '19 09:11 medikoo