oc icon indicating copy to clipboard operation
oc copied to clipboard

OC is very strict on server-side dependency resolving strategies

Open matteofigus opened this issue 7 years ago • 28 comments

As you all know, OC is very strict about allowing components to use node dependencies on the server.js.

Currently, the registry has a whitelist of allowed dependencies, and its discretion and responsibility of registry owners to ensure components load correctly with the installed versions.

Some contributors put together a beautiful document with ideas about how to make this better: https://drive.google.com/file/d/0B_XTziQjKDm4aFB3TjdnOEhrOE0/view

Today as 2017-02-24 we had a meeting at OpenTable where we discussed the purposed 5 approaches. This is the outcome: https://docs.google.com/document/d/17n9uaObBOwcZuM7rLmr50ZnQACehTuadOncPERzNnh4/edit?usp=sharing

@mattiaerre will organise and lead an open retrospective that we'll do via google hangouts, anyone free to attend - to discuss further actions.

/cc @debopamsengupta @ferewuz @cezarp @matthewdavidson @opentable/new-markets @opentable/open-components-team

matteofigus avatar Feb 24 '17 18:02 matteofigus

Hello all, I've just created a Doodle we can use in order to see what's the best time for us to schedule this Google Hangouts:

http://doodle.com/poll/x35kruy2q93xxm6s

please feel free to share it w/ who might be interested.

If you are not sure about time zones and stuff this can be a very useful link:

http://www.worldtimebuddy.com/?pl=1&lid=5391959,2643743&h=2643743

//cc @debopamsengupta @ferewuz @cezarp @matthewdavidson @opentable/new-markets @opentable/open-components-team @navamgupta

mattiaerre avatar Feb 27 '17 17:02 mattiaerre

Hello, as per the poll, it looks like the day/time of this workshop is going to be:

Monday 06, March 2017 800a-900a PST / 400p-500p GMT

I will send you a Google Hangouts link beforehand. As well as some more details about the format of this workshop so that you can prepare yourself upfront.

save the date!

//cc @debopamsengupta @ferewuz @cezarp @matthewdavidson @opentable/new-markets @opentable/open-components-team @navamgupta

mattiaerre avatar Mar 03 '17 06:03 mattiaerre

OC dependency management workshop

During this workshop, we will discuss how to improve dependency management at the OC registry level. In particular, we will try to answer the following questions:

  • What speeds me up?

  • What slows me down?

Ideally, we would like to look back at how we are using OC and understand if there are things that are accelerating our team's workflow and if there are others that are anchoring it instead.

When?

Monday 06, March 2017 800a-900a PST / 400p-500p GMT

Prerequisites

It'd be good if you:

  • Can read the documents that @matteofigus added to the very first comment on this issue

  • Can start thinking about the 2 questions above so that we can be more efficient during the workshop

Board

There is a Trello board that we will use during the workshop; let me know if you have got any issue when you try to access it (it should be public but you'll never know).

Google Hangout

@matteofigus can you confirm the link of the Google Hangout you would like to use for this meeting?

//cc @debopamsengupta @ferewuz @cezarp @matthewdavidson @opentable/new-markets @opentable/open-components-team @navamgupta

mattiaerre avatar Mar 04 '17 22:03 mattiaerre

@matteofigus @mattiaerre Hey guys, is there a Hangouts link posted anywhere for the meeting?

vanclevstik avatar Mar 06 '17 15:03 vanclevstik

On the top right corner of the Trello Board we are going to use there is a link to a Google hangout; we can use that one unless @matteofigus has got another public one he would like to use instead

screen shot 2017-03-06 at 07 46 01

https://hangouts.google.com/hangouts/_/event/trello.com/b/neeWL9uR-zesty-mosquito

//cc @debopamsengupta @ferewuz @cezarp @matthewdavidson @opentable/new-markets @opentable/open-components-team @navamgupta

mattiaerre avatar Mar 06 '17 15:03 mattiaerre

I confirm, we will use this Google Hangout link:

https://hangouts.google.com/hangouts/_/2xe5soc4enedhpdxi5aq26friye

//cc @debopamsengupta @ferewuz @cezarp @matthewdavidson @opentable/new-markets @opentable/open-components-team @navamgupta

mattiaerre avatar Mar 06 '17 15:03 mattiaerre

email: [email protected]

vanclevstik avatar Mar 06 '17 16:03 vanclevstik

[email protected]

mh52 avatar Mar 06 '17 16:03 mh52

[email protected]

danrr avatar Mar 06 '17 16:03 danrr

[email protected]~ [email protected] for the trello board pls

furybyname avatar Mar 06 '17 16:03 furybyname

Apologies guys got stuck resolving another problem. Looking forward to your final decision :)

navamgupta avatar Mar 06 '17 16:03 navamgupta

no problem at all @navamgupta I will send you guys ASAP some notes re this meeting. I hope you guys enjoyed as much as I did. thanks for your valuable feedback

mattiaerre avatar Mar 06 '17 23:03 mattiaerre

Hello, I've created this Google doc w/ notes re this meeting; you can find it here:

https://docs.google.com/a/opentable.com/document/d/1I9hYSD86XAp5iiIdhrrWv8wGx5ib-4TS5KkAIcCM9Uc/edit?usp=sharing

let me know if you cannot access it and also feel free to add your comments.

mattiaerre avatar Mar 07 '17 04:03 mattiaerre

Hi @mattiaerre we're unable to access the document, possible to grant us permission ?

debopamsengupta avatar Mar 07 '17 10:03 debopamsengupta

@debopamsengupta you should be able to access it now; as you've done you need to request access. Let me know if you are still unable to access it.

mattiaerre avatar Mar 07 '17 15:03 mattiaerre

As promised, this is my proposal (simplified):

  1. No limits on making people webpack their own deps into the server.js (but we may want to expose the possibility to hook into the current webpack process in order to make this configurable - as part of a separate github issue - anyone free to open a issue/PR)
  2. API for configuration.dependencies changes from (non breaking change) dependencies: ["lodash", "request"] to dependencies: ["lodash", "[email protected]", "[email protected]"]
  3. dependencies won't need to be declared in oc-registry package.json (it is not possible to declare multiple versions)
  4. registry on start will install deps into oc_node_modules and will use components' package.json to resolve proper version
  5. the dependencies resolver will be published as cli npm module as well, so that will be possible be performed as oc-registry postInstall script and possibly facilitate deployments (the registry on start will need to just validate all is in place instead of installing)

Goals are:

  1. Unblocking component creators to use new versions of dependencies and make oc-registry maintainers able to do it
  2. Not break old components depending on old versions and guarantee immutability on already published components in terms of server-side behaviour during oc-registry deployments
  3. Keep memory usage as low as possible and performance at best as possible

Comments on this? Specifically,

  1. Anyone disagrees about having this feature?
  2. Anyone with better ideas about the proposed approach?
  3. Everybody agrees about the API?

matteofigus avatar Mar 10 '17 13:03 matteofigus

Nice overview of the proposal! Overall I think it's a good balance between registry complexity / maintenance and component creator happiness. Your point about possibly opening up the webpack bundling process to consumers is interesting. I've seen similar approaches in next.js and react-storybook - they could be good inspiration.

Anyways, 👍 from me.

matthewdavidson avatar Mar 13 '17 09:03 matthewdavidson

@matteofigus One use case a colleague of mine pointed out is we overload the component's package.json for defining dev dependencies for unit testing and building (gulp stuff for various things like minify concat among others) files. These dependencies are not really mentioned in the registry because these are more pre publish tasks rather than inside registry tasks. Till now registry did not care for the component package json's dependencies but since it will care now do you forsee things breaking in our setup? Do you suggest we change our setup anticipating breakage?

navamgupta avatar Mar 21 '17 20:03 navamgupta

@navamgupta I don't think this will be a problem if declared as devDependencies. This should affect only dependencies and only if really "required/imported" inside the server.js (as webpack bundling includes tree shaking so unreferenced dependencies should just be ignored).

matteofigus avatar Mar 21 '17 20:03 matteofigus

@matteofigus in order to be super clear it is possible to create a couple of example use cases where we can see what kind of problem we are trying to solve here? thanks 😅

mattiaerre avatar Mar 22 '17 16:03 mattiaerre

@matteofigus are we ready to start integrating the dependency-resolving functionality into OC ?

debopamsengupta avatar May 08 '17 10:05 debopamsengupta

I think we should be able to using https://github.com/debopamsengupta/requirey for that. The API I suggest is:

  1. Configuration from array of dependencies goes to array of dependencies including (optionally) the version. Example: ['lodash', '[email protected]', '[email protected]']
  2. OC registry will do the install when it starts, skipping the operation if the deps are already installed
  3. I suggest creating a new functionality to do the pre-install (useful for deploying registry on docker containers) of those dependencies. So I would extend the registry instance: https://github.com/opentable/oc/wiki/Registry#api to include a new operation oc.installComponentsDependencies():
const oc = require('oc');
const registry = new oc.Registry(configuration);
registry.installComponentsDependencies(callback);

^ this could be used as own script to be run as npm post-install for the registry app.

matteofigus avatar May 08 '17 13:05 matteofigus

@mattiaerre I think the easiest use case I can think about is the registry using [email protected] which has a security issue and want to upgrade to [email protected] which fixes but with breaking changes. We want to allow components to use both, possibly to safely migrate to foo@3, and then possibly remove foo@2 later. (Applies not only to security issues but to any dependency that introduces breaking changes.)

matteofigus avatar May 08 '17 13:05 matteofigus

@matteofigus let me know if there is anything else I can do re this conversation; happy to help w/ the code also

mattiaerre avatar May 08 '17 15:05 mattiaerre

Since version 6 of npm you can use aliases for packages, with syntax like this: npm install <alias>@npm:<name>:

This could be a solution, instead of using npm-install-version, which right now does not even work. You can keep declaring your different versions on package.json and there is no need to install them after starting the registry, you would just need the logic to identify aliases on the dependencies array.

ricardo-devis-agullo avatar Aug 27 '21 14:08 ricardo-devis-agullo