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

OC is very strict on server-side dependency resolving strategies #390

Open
matteofigus opened this issue Feb 24, 2017 · 28 comments
Open

OC is very strict on server-side dependency resolving strategies #390

matteofigus opened this issue Feb 24, 2017 · 28 comments
Assignees
Milestone

Comments

@matteofigus
Copy link
Member

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

@mattiaerre mattiaerre self-assigned this Feb 24, 2017
@mattiaerre
Copy link
Member

mattiaerre commented Feb 27, 2017

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
Copy link
Member

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
Copy link
Member

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

@vanclevstik
Copy link

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

@mattiaerre
Copy link
Member

mattiaerre commented Mar 6, 2017

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
Copy link
Member

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

@debopamsengupta
Copy link
Member

[email protected]

@vanclevstik
Copy link

email: [email protected]

@matthewdavidson
Copy link
Contributor

[email protected]

@NimaSoroush
Copy link
Collaborator

[email protected]

@mh52
Copy link

mh52 commented Mar 6, 2017

[email protected]

@danrr
Copy link

danrr commented Mar 6, 2017

[email protected]

@furybyname
Copy link

furybyname commented Mar 6, 2017

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

@navamgupta
Copy link

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

@mattiaerre
Copy link
Member

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
Copy link
Member

mattiaerre commented Mar 7, 2017

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.

@debopamsengupta
Copy link
Member

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

@mattiaerre
Copy link
Member

@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.

@matteofigus
Copy link
Member Author

matteofigus commented Mar 10, 2017

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 matteofigus added this to the v1.0.0 milestone Mar 12, 2017
@matthewdavidson
Copy link
Contributor

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.

@navamgupta
Copy link

@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?

@matteofigus
Copy link
Member Author

@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).

@mattiaerre
Copy link
Member

@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 😅

@debopamsengupta
Copy link
Member

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

@matteofigus
Copy link
Member Author

matteofigus commented May 8, 2017

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
Copy link
Member Author

@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.)

@mattiaerre
Copy link
Member

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

@ricardo-devis-agullo
Copy link
Collaborator

ricardo-devis-agullo commented Aug 27, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests