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

refactor: untangling lib/index.js part 1, monitor #443

Merged
merged 1 commit into from
Apr 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"build": "tsc",
"build-watch": "tsc -w",
"eslint": "eslint -c .eslintrc src",
"find-circular": "npm run build && madge --circular ./dist",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registry uses  madge --extensions js,ts --circular ./src ./test, not sure if there's a benefit of doing that, just sharing :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I've copied this from the registry. It looks like madge is not 100% comfy with Typepscript, so the result was that the cycles were found twice - once in src, once in dist.

"tslint": "tslint --project tsconfig.json --format stylish --exclude **/src/**/*.js",
"prepare": "npm run build",
"tap": "tap test/*.test.* -Rspec --timeout=300 --node-arg=-r --node-arg=ts-node/register",
Expand Down Expand Up @@ -95,6 +96,7 @@
"@types/needle": "^2.0.4",
"@types/node": "^10.0.0",
"eslint": "^5.16.0",
"madge": "^3.4.4",
"nock": "^10.0.6",
"proxyquire": "^1.7.4",
"restify": "^4.1.1",
Expand Down
6 changes: 2 additions & 4 deletions src/cli/commands/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as _ from 'lodash';
import * as fs from 'then-fs';
import {exists as apiTokenExists} from '../../lib/api-token';
import snyk = require('../../lib/'); // TODO(kyegupov): fix import
import {monitor as snykMonitor} from '../../lib/monitor';
import * as config from '../../lib/config';
import * as url from 'url';
import chalk from 'chalk';
Expand All @@ -14,8 +15,7 @@ import * as detect from '../../lib/detect';
import * as plugins from '../../lib/plugins';
import ModuleInfo = require('../../lib/module-info'); // TODO(kyegupov): fix import
import * as docker from '../../lib/docker-promotion';
import { MonitorError } from '../../lib/monitor';
import {SingleDepRootResult, MultiDepRootsResult, isMultiResult} from '../../lib/types';
import {SingleDepRootResult, MultiDepRootsResult, isMultiResult, MonitorError } from '../../lib/types';

const SEPARATOR = '\n-------------------------------------------------------\n';

Expand Down Expand Up @@ -146,8 +146,6 @@ async function monitor(...args0: any[]): Promise<any> {
// Post the project dependencies to the Registry
const monOutputs: string[] = [];
for (const depRootDeps of perDepRootResults) {
// TODO(kyegupov): make snyk.monitor typed by converting src/lib/index.js to TS
const snykMonitor = snyk.monitor as any as (path, meta, depRootDeps) => Promise<any>;
const res = await promiseOrCleanup(
snykMonitor(path, meta, depRootDeps),
spinner.clear(postingMonitorSpinnerLabel));
Expand Down
3 changes: 2 additions & 1 deletion src/cli/commands/protect/wizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const getVersion = require('../version');
const allPrompts = require('./prompts');
const answersToTasks = require('./tasks');
const snyk = require('../../../lib/');
const snykMonitor = require('../../../lib/monitor').monitor;
const isCI = require('../../../lib/is-ci');
const protect = require('../../../lib/protect');
const authorization = require('../../../lib/authorization');
Expand Down Expand Up @@ -519,7 +520,7 @@ function processAnswers(answers, policy, options) {

return info.inspect(cwd, targetFile, options)
.then(spinner(lbl))
.then(snyk.monitor.bind(null, cwd, meta))
.then(snykMonitor.bind(null, cwd, meta))
// clear spinner in case of success or failure
.then(spinner.clear(lbl))
.catch((error) => {
Expand Down
3 changes: 2 additions & 1 deletion src/lib/capture.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module.exports = captureRequires;
var path = require('path');
var modules = [];
var snyk = require('../lib');
var snykMonitor = require('../lib/monitor').monitor;

function captureRequires() {
var timer = null;
Expand Down Expand Up @@ -42,7 +43,7 @@ function resolve() {

// then collect all the package deps and post a monitor
var cwd = path.resolve(candidate, '..');
snyk.modules(cwd).then(snyk.monitor.bind(null, cwd, {
snyk.modules(cwd).then(snykMonitor.bind(null, cwd, {
method: 'require',
}));
}
9 changes: 8 additions & 1 deletion src/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ module.exports = snyk;
var cluster = require('cluster');
var snykConfig = require('./config');

// This module exports a function with properties attached.
// The properties represent other modules - so, it's some kind of "world object"
// that is used to indirectly import modules.
// The function modifies the object and returns it.
// This also introduces some circular imports.

// TODO(kyegupov): untangle this, resolve circular imports, convert to Typescript

function snyk(options) {
if (!options) {
options = {};
Expand Down Expand Up @@ -50,7 +58,6 @@ Object.defineProperty(snyk, 'api', {

snyk.modules = require('./modules');
snyk.test = require('./snyk-test');
snyk.monitor = require('./monitor');
snyk.bus = require('./bus');
snyk.policy = require('snyk-policy');
snyk.isRequired = true; // changed to false when loaded via cli
Expand Down
11 changes: 2 additions & 9 deletions src/lib/monitor.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
module.exports = monitor;

import * as snyk from '../lib';
import {exists as apiTokenExists} from './api-token';
import * as request from './request';
Expand All @@ -8,12 +6,7 @@ import * as os from 'os';
import * as _ from 'lodash';
import * as isCI from './is-ci';
import * as analytics from './analytics';
import { SingleDepRootResult } from './types';

export class MonitorError extends Error {
public code?: number;
public userMessage?: string;
}
import { SingleDepRootResult, MonitorError } from './types';

// TODO(kyegupov): clean up the type, move to snyk-cli-interface repository

Expand Down Expand Up @@ -42,7 +35,7 @@ interface Meta {
projectName: string;
}

function monitor(root, meta, info: SingleDepRootResult) {
export function monitor(root, meta, info: SingleDepRootResult): Promise<any> {
const pkg = info.package;
const pluginMeta = info.plugin;
let policyPath = meta['policy-path'];
Expand Down
5 changes: 5 additions & 0 deletions src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,8 @@ export interface MultiDepRootsResult {
export function isMultiResult(pet: SingleDepRootResult | MultiDepRootsResult): pet is MultiDepRootsResult {
return !!(pet as MultiDepRootsResult).depRoots;
}

export class MonitorError extends Error {
public code?: number;
public userMessage?: string;
}