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

misc(gulp): rename output files #6179

Merged
merged 5 commits into from
Oct 10, 2018
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
4 changes: 2 additions & 2 deletions docs/releasing.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ git push --tags
echo "Rebuild extension and viewer to get the latest, tagged master commit"
yarn build-all;

# zip the extension files, but remove lh-background as it's not needed
cd lighthouse-extension; command rm -f dist/scripts/lighthouse-background.js; gulp package; cd ..
Copy link
Member

Choose a reason for hiding this comment

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

we should really update gulp package to do this for us. I've definitely never remembered to delete the other files :):)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i'll add a new PR that fixes this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

# zip the extension files, but remove lh-dt-bundle & lh-lr-bundle as it's not needed
cd lighthouse-extension; command rm -f dist/scripts/lighthouse-dt-bundle.js dist/scripts/lighthouse-lr-bundle.js; gulp package; cd ..
echo "Go here: https://chrome.google.com/webstore/developer/edit/blipmdconlkpinefehnmjammfjpmpbjk "
echo "Upload the package zip to CWS dev dashboard"

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/connections/extension.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class ExtensionConnection extends Connection {
}

/**
* Used by lighthouse-ext-background to kick off the run on the current page
* Used by extension-entry to kick off the run on the current page
* @return {Promise<string>}
*/
getCurrentTabURL() {
Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/scripts/roll-to-devtools.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ fi
report_dir="lighthouse-core/report/html"
fe_lh_dir="$frontend_dir/audits2/lighthouse"

lh_bg_js="lighthouse-extension/dist/scripts/lighthouse-background.js"
lh_bg_js="lighthouse-extension/dist/scripts/lighthouse-dt-bundle.js"
lh_worker_dir="$frontend_dir/audits2_worker/lighthouse"

# copy report files
cp -pPR $report_dir/{report-styles.css,templates.html,renderer} "$fe_lh_dir"
echo -e "\033[32m ✓\033[39m Report renderer files copied."

# copy lighthouse-background (potentially stale)
cp -pPR "$lh_bg_js" "$lh_worker_dir/lighthouse-background.js"
echo -e "\033[96m ✓\033[39m (Potentially stale) lighthouse-background copied."
# copy lighthouse-dt-bundle (potentially stale)
cp -pPR "$lh_bg_js" "$lh_worker_dir/lighthouse-dt-bundle.js"
echo -e "\033[96m ✓\033[39m (Potentially stale) lighthouse-dt-bundle copied."
2 changes: 1 addition & 1 deletion lighthouse-extension/app/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"default_locale": "en",
"background": {
"scripts": [
"scripts/lighthouse-ext-background.js"
"scripts/lighthouse-ext-bundle.js"
],
"persistent": false
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function listenForStatus(listenCallback) {
}

if (typeof module !== 'undefined' && module.exports) {
// export for lighthouse-ext-background to require (via browserify).
// export for extension-entry to require (via browserify).
module.exports = {
getDefaultConfigForCategories,
runLighthouseInWorker,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const lighthouse = require('../../../lighthouse-core/index');
const background = require('./lighthouse-background');
const background = require('./devtools-entry');
Copy link
Member

Choose a reason for hiding this comment

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

it's unfortunate to have to keep calling this background, but I can't think of a better name right now :)

Copy link
Collaborator Author

@wardpeet wardpeet Oct 10, 2018

Choose a reason for hiding this comment

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

if I move getDefaultConfigForCategories & getDefaultCategories to it's own file called one of them below I could rename the variable which gives more meaning to it.

  • config.js,
  • helpers/config.js
  • optionsHelper.js
  • runOptions.js
  • lighthouseOptions.js
  • ...

Copy link
Member

Choose a reason for hiding this comment

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

yeah, I think that makes sense, but I think we should just wait on it (at least until LR shakes out and we can fully see the shape of these files for the medium-term future).


const ExtensionProtocol = require('../../../lighthouse-core/gather/connections/extension');
const log = require('lighthouse-logger');
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-extension/app/src/popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

/** @typedef {typeof import('./lighthouse-ext-background.js') & {console: typeof console}} BackgroundPage */
/** @typedef {typeof import('./extension-entry.js') & {console: typeof console}} BackgroundPage */

/**
* Error strings that indicate a problem in how Lighthouse was run, not in
Expand Down Expand Up @@ -234,7 +234,7 @@ async function initPopup() {

/**
* Really the Window of the background page, but since we only want what's exposed
* on window in lighthouse-ext-background.js, use its module API as the type.
* on window in extension-entry.js, use its module API as the type.
* @type {BackgroundPage}
*/
const background = await new Promise(resolve => chrome.runtime.getBackgroundPage(resolve));
Expand Down
43 changes: 33 additions & 10 deletions lighthouse-extension/gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
'use strict';

const fs = require('fs');
const path = require('path');
// HACK: patch astw before it's required to use acorn with ES2018
// We add the right acorn version to package.json deps, resolve the path to it here,
// and then inject the modified require statement into astw's code.
Expand Down Expand Up @@ -36,9 +37,18 @@ const distDir = 'dist';

// list of all consumers we build for (easier to understand which file is used for which)
const CONSUMERS = {
DEVTOOLS: 'lighthouse-background.js',
EXTENSION: 'lighthouse-ext-background.js',
LIGHTRIDER: 'lighthouse-lr-background.js',
DEVTOOLS: {
src: 'devtools-entry.js',
dist: 'lighthouse-dt-bundle.js',
},
EXTENSION: {
src: 'extension-entry.js',
dist: 'lighthouse-ext-bundle.js',
},
LIGHTRIDER: {
src: 'lightrider-entry.js',
dist: 'lighthouse-lr-bundle.js',
},
};

const VERSION = pkg.version;
Expand All @@ -60,8 +70,10 @@ const computedArtifacts = LighthouseRunner.getComputedGathererList()
const locales = fs.readdirSync('../lighthouse-core/lib/i18n/locales/')
.map(f => require.resolve(`../lighthouse-core/lib/i18n/locales/${f}`));

const isDevtools = file => file.endsWith(CONSUMERS.DEVTOOLS);
const isExtension = file => file.endsWith(CONSUMERS.EXTENSION);
const isDevtools = file =>
file.endsWith(CONSUMERS.DEVTOOLS.src);
const isExtension = file =>
file.endsWith(CONSUMERS.EXTENSION.src);

gulp.task('extras', () => {
return gulp.src([
Expand Down Expand Up @@ -109,7 +121,7 @@ gulp.task('chromeManifest', () => {
const manifestOpts = {
buildnumber: false,
background: {
target: `scripts/${CONSUMERS.EXTENSION}`,
target: `scripts/${CONSUMERS.EXTENSION.dist}`,
},
};
return gulp.src('app/manifest.json')
Expand All @@ -127,10 +139,10 @@ function applyBrowserifyTransforms(bundle) {
}

gulp.task('browserify-lighthouse', () => {
const consumerSources = Object.values(CONSUMERS).map(consumer => `app/src/${consumer}`);
const consumerSources = Object.values(CONSUMERS).map(consumer => `app/src/${consumer.src}`);
return gulp.src(consumerSources, {read: false})
.pipe(tap(file => {
let bundle = browserify(file.path, {debug: true}); // for sourcemaps
let bundle = browserify(file.path); // , {debug: true}); // for sourcemaps
bundle = applyBrowserifyTransforms(bundle);

// scripts will need some additional transforms, ignores and requires…
Expand Down Expand Up @@ -176,6 +188,18 @@ gulp.task('browserify-lighthouse', () => {
// Inject the new browserified contents back into our gulp pipeline
file.contents = bundle.bundle();
}))
.pipe(debug({title: ''}))
.pipe(tap(file => {
// rename our bundles
const basename = path.basename(file.path);

// find the dist file of the given file
const consumer = Object.values(CONSUMERS)
.find(consumer => consumer.src === basename);

file.path = file.path.replace(consumer.src, consumer.dist);
}))
.pipe(debug({title: 'renamed into:'}))
.pipe(gulp.dest('app/scripts'))
.pipe(gulp.dest('dist/scripts'));
});
Expand Down Expand Up @@ -212,7 +236,7 @@ gulp.task('compilejs', () => {
// sourceMaps: 'both'
};

const compiledSources = Object.values(CONSUMERS).map(consumer => `dist/scripts/${consumer}`);
const compiledSources = Object.values(CONSUMERS).map(consumer => `dist/scripts/${consumer.dist}`);
return gulp.src(compiledSources)
.pipe(tap(file => {
const minified = babel.transform(file.contents.toString(), opts).code;
Expand All @@ -229,7 +253,6 @@ gulp.task('clean', () => {
);
});


gulp.task('watch', ['browserify', 'html'], () => {
livereload.listen();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
'use strict';

const assert = require('assert');
const lhBackground = require('../../../app/src/lighthouse-lr-background.js');
const lhBackground = require('../../../app/src/lightrider-entry.js');
const LHError = require('../../../../lighthouse-core/lib/lh-error.js');

/* eslint-env mocha */

describe('lighthouse-lr-background', () => {
describe('lightrider-entry', () => {
describe('#runLighthouseInLR', () => {
it('returns a runtimeError LHR when lighthouse throws a runtimeError', async () => {
const connectionError = new LHError(LHError.errors.FAILED_DOCUMENT_REQUEST);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
},
"bundlesize": [
{
"path": "./lighthouse-extension/dist/scripts/lighthouse-background.js",
"path": "./lighthouse-extension/dist/scripts/lighthouse-ext-bundle.js",
"threshold": "520 Kb"
},
{
Expand Down