endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
812 stars 71 forks source link

remove package cycles #2196

Open turadg opened 6 months ago

turadg commented 6 months ago

What is the Problem Being Solved?

Lerna complains,

lerna WARN ECYCLE Dependency cycles detected, you should fix these!
lerna WARN ECYCLE @endo/compartment-mapper -> ses -> @endo/compartment-mapper
lerna WARN ECYCLE @endo/static-module-record -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/compartment-mapper) -> @endo/static-module-record
lerna WARN ECYCLE (nested cycle: @endo/static-module-record -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/compartment-mapper) -> @endo/static-module-record) -> (nested cycle: @endo/static-module-record -> (nested cycle: @endo/compartment-mapper -> ses -> @endo/compartment-mapper) -> @endo/static-module-record)

Since https://github.com/endojs/endo/pull/1633 this repo doesn't have any cycles in its dependencies graph, but it still has cycles when including devDependencies.

Description of the Design

Refactor devDependencies to remove the cycles.

Security Considerations

Scaling Considerations

Test Plan

Compatibility Considerations

Upgrade Considerations

kriskowal commented 6 months ago

I’m not sure that it is possible to solve this problem. There is a certain meta-circularity in the dev toolchain. SES is at the bottom of the layer cake, but it reaches up to compartment-mapper/bundle.js to build the “rollups” in dist/* when it is published to npm.

turadg commented 6 months ago

I’m not sure that it is possible to solve this problem. There is a certain meta-circularity in the dev toolchain. SES is at the bottom of the layer cake, but it reaches up to compartment-mapper/bundle.js to build the “rollups” in dist/* when it is published to npm.

Does ses need the local version of component-mapper/bundle.js? If it's just using it to bundle, it could pull from NPM.

If ses has to depend on component-mapper then the parts of SES that compartment-mapper depends on could be moved into a lower dependency like @endo/ses-core.

kriskowal commented 5 months ago

Compartment Mapper depends on Compartment but doesn’t depend on Lockdown. There is a possibility that we could separate the Compartment shim. Or, we could decompose Compartment into the underlying Module Harmony machinery for ModuleSource (StaticModuleRecord) and Module instance (to drive the dependency trampoline without necessarily executing.

erights commented 2 months ago

https://github.com/endojs/endo/pull/2356 is a failed experiment to make some progress on this.