This repository has been archived by the owner on Dec 8, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 28
Serve and open built files #219
Merged
Merged
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
7bc54be
Allowing mechanism for serving built files
a88ab54
Fixed merge conflicts from master
afae356
Cleaned up bad merge
c60e9ae
Fixed linting errors
8a8067a
Merge branch 'master' into serve-build
e7101f1
Merge branch 'master' into serve-build
047b223
Returning from build. Removing Protractor warning
a21343d
Merge branch 'serve-build' of github.com:blackbaud/skyux-builder into…
d1a54ea
Moved files. Fixed bug causing test failure
8a0808c
Manually setting name if launching after build
8947d70
Using different server to allow for better redirects
fb4f88e
Merged conflicts
Blackbaud-SteveBrush 8513ffd
Merge branch 'master' into serve-build
8c48663
Cleanup
7b11e75
Fixed conflicts
8fdf1a6
Cleaned up linting
a7a7781
Fixed mock
66f72e9
Merge branch 'master' into serve-build
2d3cb65
Cleaned up static directory
c0e743b
Merge branch 'serve-build' of github.com:blackbaud/skyux-builder into…
53dfb1c
Merged master
503c7d6
Merge branch 'master' into serve-build
7d418a5
Merge branch 'master' into serve-build
Blackbaud-SteveBrush ced2832
Removed unnecessary forward slash. Fixed logging.
fd07c81
Merge branch 'master' into serve-build
dc29453
Separated launch from serve functionality. Defaulting launch to host
70e7479
Merge branch 'serve-build' of github.com:blackbaud/skyux-builder into…
2085ebb
Refactored to use --serve and correctly set assets
7c6c298
Always returning promise (if lint passes) from build. Fixed e2e branc…
bdc59f5
Cleaned up promises
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,16 +4,15 @@ | |
const glob = require('glob'); | ||
const path = require('path'); | ||
const spawn = require('cross-spawn'); | ||
const logger = require('../utils/logger'); | ||
const portfinder = require('portfinder'); | ||
const HttpServer = require('http-server'); | ||
const selenium = require('selenium-standalone'); | ||
|
||
const build = require('./build'); | ||
const server = require('./utils/server'); | ||
const logger = require('../utils/logger'); | ||
|
||
// Disable this to quiet the output | ||
const spawnOptions = { stdio: 'inherit' }; | ||
|
||
let httpServer; | ||
let seleniumServer; | ||
let start; | ||
|
||
|
@@ -45,18 +44,13 @@ function killServers(exitCode) { | |
seleniumServer = null; | ||
} | ||
|
||
if (httpServer) { | ||
logger.info('Closing http server'); | ||
httpServer.close(); | ||
httpServer = null; | ||
} | ||
|
||
// Catch protractor's "Kitchen Sink" error. | ||
if (exitCode === 199) { | ||
logger.warn('Supressing protractor\'s "kitchen sink" error 199'); | ||
exitCode = 0; | ||
} | ||
|
||
server.stop(); | ||
logger.info(`Execution Time: ${(new Date().getTime() - start) / 1000} seconds`); | ||
logger.info(`Exiting process with ${exitCode}`); | ||
process.exit(exitCode || 0); | ||
|
@@ -91,6 +85,7 @@ function spawnProtractor(chunks, port, skyPagesConfig) { | |
protractorPath, | ||
[ | ||
getProtractorConfigPath(), | ||
`--disableChecks`, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI, adding this check to solve the "unknown arguments" warning (soon to be error) that's shown. https://github.com/angular/protractor/blob/master/CHANGELOG.md#features-3 |
||
`--baseUrl ${skyPagesConfig.skyux.host.url}`, | ||
`--params.localUrl=https://localhost:${port}`, | ||
`--params.chunks=${JSON.stringify(chunks)}`, | ||
|
@@ -150,42 +145,6 @@ function spawnSelenium() { | |
}); | ||
} | ||
|
||
/** | ||
* Spawns the httpServer | ||
*/ | ||
function spawnServer() { | ||
return new Promise((resolve, reject) => { | ||
logger.info('Requesting open port...'); | ||
|
||
httpServer = HttpServer.createServer({ | ||
root: 'dist/', | ||
cors: true, | ||
https: { | ||
cert: path.resolve(__dirname, '../', 'ssl', 'server.crt'), | ||
key: path.resolve(__dirname, '../', 'ssl', 'server.key') | ||
}, | ||
logFn: (req, res, err) => { | ||
if (err) { | ||
reject(err); | ||
return; | ||
} | ||
} | ||
}); | ||
|
||
portfinder | ||
.getPortPromise() | ||
.then(port => { | ||
logger.info(`Open port found: ${port}`); | ||
logger.info('Starting web server...'); | ||
httpServer.listen(port, 'localhost', () => { | ||
logger.info('Web server running.'); | ||
resolve(port); | ||
}); | ||
}) | ||
.catch(reject); | ||
}); | ||
} | ||
|
||
/** | ||
* Spawns the build process. Captures the config used. | ||
*/ | ||
|
@@ -218,7 +177,7 @@ function e2e(argv, skyPagesConfig, webpack) { | |
return killServers(0); | ||
} | ||
|
||
spawnServer() | ||
server.start() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So much cleaner; love it. |
||
.then((port) => { | ||
argv.assets = 'https://localhost:' + port; | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
/*jslint node: true */ | ||
'use strict'; | ||
|
||
const util = require('util'); | ||
const open = require('open'); | ||
const logger = require('winston'); | ||
const hostUtils = require('../../utils/host-utils'); | ||
const skyPagesConfigUtil = require('../../config/sky-pages/sky-pages.config'); | ||
|
||
/** | ||
* Returns the querystring base for parameters allowed to be passed through. | ||
* PLEASE NOTE: The method is nearly duplicated in `runtime/params.ts`. | ||
* @name getQueryStringFromArgv | ||
* @param {Object} argv | ||
* @param {SkyPagesConfig} skyPagesConfig | ||
* @returns {string} | ||
*/ | ||
function getQueryStringFromArgv(argv, skyPagesConfig) { | ||
|
||
let found = []; | ||
skyPagesConfig.skyux.params.forEach(param => { | ||
if (argv[param]) { | ||
found.push(`${param}=${encodeURIComponent(argv[param])}`); | ||
} | ||
}); | ||
|
||
if (found.length) { | ||
return `?${found.join('&')}`; | ||
} | ||
|
||
return ''; | ||
} | ||
|
||
function browser(argv, skyPagesConfig, stats, port) { | ||
|
||
const queryStringBase = getQueryStringFromArgv(argv, skyPagesConfig); | ||
let localUrl = util.format( | ||
'https://localhost:%s%s', | ||
port, | ||
skyPagesConfigUtil.getAppBase(skyPagesConfig) | ||
); | ||
|
||
let hostUrl = hostUtils.resolve( | ||
queryStringBase, | ||
localUrl, | ||
stats.toJson().chunks, | ||
skyPagesConfig | ||
); | ||
|
||
// Edge uses a different technique (protocol vs executable) | ||
if (argv.browser === 'edge') { | ||
const edge = 'microsoft-edge:'; | ||
argv.browser = undefined; | ||
hostUrl = edge + hostUrl; | ||
localUrl = edge + localUrl; | ||
} | ||
|
||
// Browser defaults to launching host | ||
argv.launch = argv.launch || 'host'; | ||
|
||
switch (argv.launch) { | ||
case 'local': | ||
|
||
// Only adding queryStringBase to the message + local url opened, | ||
// Meaning doesn't need those to communicate back to localhost | ||
localUrl += queryStringBase; | ||
|
||
logger.info(`Launching Local URL: ${localUrl}`); | ||
open(localUrl, argv.browser); | ||
break; | ||
case 'host': | ||
logger.info(`Launching Host URL: ${hostUrl}`); | ||
open(hostUrl, argv.browser); | ||
break; | ||
} | ||
} | ||
|
||
module.exports = browser; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/*jslint node: true */ | ||
'use strict'; | ||
|
||
const fs = require('fs'); | ||
const path = require('path'); | ||
const logger = require('winston'); | ||
const portfinder = require('portfinder'); | ||
const express = require('express'); | ||
const https = require('https'); | ||
const cors = require('cors'); | ||
|
||
const app = express(); | ||
|
||
let server; | ||
|
||
/** | ||
* Starts the httpServer | ||
* @name start | ||
*/ | ||
function start(root) { | ||
return new Promise((resolve, reject) => { | ||
|
||
const dist = path.resolve(process.cwd(), 'dist'); | ||
|
||
logger.info('Creating web server'); | ||
app.use(cors()); | ||
|
||
logger.info(`Exposing static directory: ${dist}`); | ||
app.use(express.static(dist)); | ||
if (root) { | ||
logger.info(`Mapping server requests from ${root} to ${dist}`); | ||
app.use(root, express.static(dist)); | ||
} | ||
|
||
const options = { | ||
cert: fs.readFileSync(path.resolve(__dirname, '../../ssl/server.crt')), | ||
key: fs.readFileSync(path.resolve(__dirname, '../../ssl/server.key')) | ||
}; | ||
|
||
server = https.createServer(options, app); | ||
server.on('error', reject); | ||
|
||
logger.info('Requesting open port...'); | ||
portfinder | ||
.getPortPromise() | ||
.then(port => { | ||
logger.info(`Open port found: ${port}`); | ||
logger.info('Starting web server...'); | ||
server.listen(port, 'localhost', () => { | ||
logger.info('Web server running.'); | ||
resolve(port); | ||
}); | ||
}) | ||
.catch(reject); | ||
}); | ||
} | ||
|
||
/** | ||
* Kills the server if it exists | ||
* @name kill | ||
*/ | ||
function stop() { | ||
if (server) { | ||
logger.info('Stopping http server'); | ||
server.close(); | ||
server = null; | ||
} | ||
} | ||
|
||
module.exports = { | ||
start: start, | ||
stop: stop | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,10 +66,10 @@ function getFileContents(filePath) { | |
switch (path.extname(filePath)) { | ||
case '.scss': | ||
contents = compileSass(filePath); | ||
break; | ||
break; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's unclear how this got in to start with, but the linter complained to be about the spacing. |
||
case '.html': | ||
contents = getHtmlContents(filePath); | ||
break; | ||
break; | ||
} | ||
|
||
contents = contents | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why we're returning the promise for
buildPromise
, but not forbuildServe
? Would it be appropriate to also catch any server errors?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also makes me wonder if we were ever handling failures during the build step. Aside from linting errors, I don't see a
process.exit
for build issues. Is that by design?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
skyux build
fails outside of linting, the ProcessExitCode plugin callsprocess.exit
.I was really only concerned with returning a promise from
buildPromise
knowing thatcli/e2e.js
importsbuild
, which wouldn't be passing theserve
command. It definitely makes sense to make the signature the same (always returning a promise) regardless of the arguments.tl;dr I'm going to push an update to always have
build
return a promise, butprocess.exit
is handled in the webpack plugin.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. That makes sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonder if I should still reject the promise, even if it's linting that fails? I'm thinking so, such that the command always returns a promise regardless of what actually failed.