Skip to content

Ability to launch SourceJS via NodeJS Script #241

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Dec 20, 2016
Merged

Ability to launch SourceJS via NodeJS Script #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a2de35f
Replace Paths to the "main application" with the path to "SourceJS" u…
Alanoll Dec 12, 2016
93ce7e9
Add a NodeJS exports function, so SourceJS can be launched from anoth…
Alanoll Dec 12, 2016
3208b83
Refactored module.exports to be more expressive.
Alanoll Dec 13, 2016
cf11b84
Missed a semi-colon declaring the function variable
Alanoll Dec 13, 2016
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
16 changes: 11 additions & 5 deletions app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,7 @@ var logErrors = function(err, req, res, next) {
app.use(logErrors);
/* /Error handling */



/* Server start */
if (!module.parent) {
var startServer = function () {
var serverOpts = global.opts.core.server;
var port = serverOpts.port;

Expand All @@ -267,7 +264,7 @@ if (!module.parent) {
if (commander.test) {
var spawn = require('cross-spawn');

spawn('./node_modules/grunt-cli/bin/grunt', [commander.postGrunt, '--port='+port], {stdio: 'inherit'})
spawn('./node_modules/grunt-cli/bin/grunt', [commander.postGrunt, '--port=' + port], { stdio: 'inherit' })
.on('close', function (code) {
if (code === 0) {
log.info('Test successful');
Expand All @@ -290,5 +287,14 @@ if (!module.parent) {
}, true);
}
}
};

/* Server start */
if (!module.parent) {
startServer();
}
/* Server start */

module.exports = {
startServer: startServer
};
2 changes: 1 addition & 1 deletion core/api/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var express = require('express');
var path = require('path');
var ParseData = require(path.join(global.pathToApp, 'core/lib/parseData'));
var utils = require(path.join(global.pathToApp, 'core/lib/utils'));
var pathToApp = path.dirname(require.main.filename);
var pathToApp = global.pathToApp;
var htmlTree = require(path.join(global.pathToApp, 'core/html-tree'));
var unflatten = require(path.join(global.pathToApp,'core/unflat'));

Expand Down
2 changes: 1 addition & 1 deletion core/loadOptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ var legacyOptionsChecker = function(options, fileName){
module.exports = function(basePath, _silent){
silent = _silent;

var pathToApp = basePath || path.dirname(require.main.filename);
var pathToApp = basePath || global.pathToApp;
Copy link
Member

@robhrt7 robhrt7 Dec 13, 2016

Choose a reason for hiding this comment

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

Using globals is considered a bad practice, since it requires some special environment setup of the callee. I'll need to double-check how these changes affect the current setup.

From other side require.main.filename is also limited, and was the reason to be replaced, since you execute SourceJS code from other location.

Copy link
Author

@Alanoll Alanoll Dec 13, 2016

Choose a reason for hiding this comment

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

Globals are indeed a bad practice.
However, beyond my reasoning for running from other location, it was also inconsistent with the rest of the SourceJS code base.

I used the global.pathToApp, since it was used by the rest of the core files (see processMd, specUtils, etc). =)

Copy link
Member

@robhrt7 robhrt7 Dec 13, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

You are right, it's just the rest of the codebase also needs to be improved in that sense :) But as I said, there's no better alternative, except passing the properties directly, as it's also done here in the main execution scenario.

But first let's discuss other SourceJS calling scenarios, since we may find a better option (following this discussion #240 (comment)).


var mergedOptions = utils.requireUncached(path.join(pathToApp, 'options'));

Expand Down
2 changes: 1 addition & 1 deletion core/middlewares/clarify.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var http = require('http');

var ejs = require(path.join(global.pathToApp, 'core/ejsWithHelpers.js'));
var trackStats = require(path.join(global.pathToApp, 'core/trackStats'));
var pathToApp = path.dirname(require.main.filename);
var pathToApp = global.pathToApp;
var specUtils = require(path.join(pathToApp, 'core/lib/specUtils'));
var ParseData = require(path.join(pathToApp, 'core/lib/parseData'));
var specsParser = require(path.join(pathToApp, 'core/lib/specPageParser'));
Expand Down
2 changes: 1 addition & 1 deletion core/routes/redirects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

var path = require('path');
var express = require('express');
var pathToApp = path.dirname(require.main.filename);
var pathToApp = global.pathToApp;

/*
*
Expand Down