feat(performance): allow creating a faster capsule #2162
Conversation
|
@imsnif please configure the circle to be able to install the package so we can see that nothing breaks |
src/constants.ts
Outdated
| @@ -4,6 +4,7 @@ import * as path from 'path'; | |||
| import format from 'string-format'; | |||
| import { PathOsBased } from './utils/path'; | |||
| import { ComponentOrigin } from './consumer/bit-map/component-map'; | |||
| process.env.MEMFS_DONT_WARN = 'true'; // suppress fs experimental warnings from memfs | |||
There was a problem hiding this comment.
i'm not sure this is the correct place to put it.
maybe in the main file (bin/bit.js)?
src/environment/isolator.ts
Outdated
| // @ts-ignore AUTO-ADDED-AFTER-MIGRATION-PLEASE-FIX! | ||
| this.capsulePackageJson = this.componentWithDependencies.component.packageJsonFile; | ||
| // @ts-ignore AUTO-ADDED-AFTER-MIGRATION-PLEASE-FIX! | ||
| this.componentRootDir = this.componentWithDependencies.component.writtenPath; | ||
| await this._addComponentsToRoot(); | ||
| logger.debug('ManyComponentsWriter, install packages on capsule'); | ||
| await this._installWithPeerOption(); | ||
| if (!opts.skipNodeModules) { |
There was a problem hiding this comment.
looks like it's the same as the installNpmPackages option. (and it's just a bug it wasn't used here).
if so, let's use the existing one?
|
Hey, @GiladShoham: so this is ready for another review round. At the time of this writing the AppVeyor tests are still running, but the e2e circle tests are passing. I'll address the AppVeyor errors if/when they come. Things I did:
|
|
✅ Build bit 1.0.8499 completed (commit 0e0dddbe01 by @imsnif) |
| @@ -83,6 +98,8 @@ export default class Isolator { | |||
| }); | |||
| } | |||
| const writeToPath = opts.writeToPath; | |||
| const installNpmPackages = typeof opts.installNpmPackages === 'undefined' ? true : opts.installNpmPackages; | |||
| // default should be true | |||
There was a problem hiding this comment.
please move the comment to be above the code line, it's above the code across the whole repo. (relevant for other comments as well)
| async _writeFileToCapsule(capsule: Capsule, file: AbstractVinyl) { | ||
| if (file.override === false) { | ||
| async _writeFileToCapsule(capsule: Capsule, file: AbstractVinyl, opts = { overwriteExisting: false }) { | ||
| if (file.override === false || opts.overwriteExisting) { |
There was a problem hiding this comment.
I don't like the flow here, it's very confusing:
- what is the different between
file.overrideandopts.overwriteExisting? where do I get each one of them? who is stronger? - the condition is very weird, it's sounds something like - if (not override or override get here). like if (false or true) enter.
(I don't have in mind a better way, but let's try thinking about something more clear)
|
❌ Build bit 1.0.8504 failed (commit c9e315b674 by @imsnif) |
|
❌ Build bit 1.0.8505 failed (commit 2e2c078bd9 by @imsnif) |
|
❌ Build bit 1.0.8521 failed (commit 7e2351dbaf by @imsnif) |
|
✅ Build bit 1.0.8536 completed (commit 59d529c3c9 by @imsnif) |
|
✅ Build bit 1.0.8541 completed (commit b86eeeae30 by @imsnif) |
Proposed Changes
This integrates the librarian package manager to bit when building components. It allows an environment to:
skipNodeModulesargument when isolating, so that the capsule will know not to install with npm/yarn.capsule.execNodeto run an executable through librarian, which patches the executablesfsmodule with the required node_modules.keepExistingCapsuleflag to bit so that the capsule will not be deleted when it is done (this is so that the next librarian installation will be cached and much faster - it can also help an environment rely on incremental builds).This change is