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.

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

Jump to bottom

Added a polyserve option doNotCompile and speed up WCT tests #168

Closed
wants to merge 3 commits into from
Closed

Added a polyserve option doNotCompile and speed up WCT tests #168

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5bd1fdf
Added a --do-not-compile flag to polyserve and use it from wct to exc…
usergenic Apr 18, 2018
25cabbb
Updated package-lock.json for polyserve
usergenic Apr 18, 2018
ab9059d
Fixed the expected URL of '/web-component-tester/browser.js' to not c…
usergenic Apr 18, 2018
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 packages/polyserve/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@types/escape-html": "0.0.20",
"@types/express": "^4.0.36",
"@types/mime": "0.0.29",
"@types/minimatch": "^3.0.3",
"@types/mz": "0.0.29",
"@types/node": "^9.6.4",
"@types/opn": "^3.0.28",
Expand All @@ -48,6 +49,7 @@
"http-proxy-middleware": "^0.17.2",
"lru-cache": "^4.0.2",
"mime": "^1.3.4",
"minimatch": "^3.0.4",
"mz": "^2.4.0",
"opn": "^3.0.2",
"pem": "^1.8.3",
Expand Down
8 changes: 8 additions & 0 deletions packages/polyserve/src/args.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ export const args: ArgDescriptor[] = [
type: String,
defaultValue: 'auto',
},
{
name: 'do-not-compile',
Copy link
Member

Choose a reason for hiding this comment

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

Now we have --compile and --do-not-compile, which might be slightly confusing. I guess we should just document the interaction between the two, especially that this takes precedence over --compile=always?

(I was also thinking we could update the --compile flag to take glob patterns (and minimatch already supports the ! operator), but then you still have the auto vs always difference that needs to be represented somehow. Not sure if this idea makes sense.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible alternate names:

  • --pass-thru
  • --do-not-modify
  • --serve-as-is
  • --as-is
  • --no-transform
  • --no-rewrite
  • --immutable
  • --static
  • --constant
  • --invariant

Copy link
Member

Choose a reason for hiding this comment

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

But do we want to disable all the transforms, or is this just about ES5 compilation?

description: 'Disables compilation for file(s) matching given pattern. ' +
Copy link
Member

Choose a reason for hiding this comment

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

Can we clarify what we mean by compilation? As currently written, it bypasses ES5 compilation, but also name-to-path transformation and AMD transformation. compile-middleware is already probably too aggressive in this regard, since the compile=never returns false from shouldTransform in the same way.

'Value given may contain glob-style wildcards such as *, **, and ?. ' +
'Use multiple --do-not-compile flag for multiple patterns.',
type: String,
multiple: true,
},
{
name: 'module-resolution',
description: 'Algorithm to use for resolving module specifiers in import ' +
Expand Down
9 changes: 8 additions & 1 deletion packages/polyserve/src/compile-middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {parse as parseContentType} from 'content-type';
import {Request, RequestHandler, Response} from 'express';
import * as fs from 'fs';
import * as LRU from 'lru-cache';
import * as minimatch from 'minimatch';
import * as path from 'path';
import {htmlTransform, jsTransform} from 'polymer-build';

Expand Down Expand Up @@ -60,7 +61,8 @@ export function babelCompile(
rootDir: string,
packageName: string,
componentUrl: string,
componentDir: string): RequestHandler {
componentDir: string,
doNotCompile: string[] = []): RequestHandler {
return transformResponse({

shouldTransform(request: Request, response: Response) {
Expand All @@ -69,6 +71,11 @@ export function babelCompile(
if (isPolyfill.test(request.url)) {
return false;
}
if (doNotCompile.length > 0 &&
!!doNotCompile.find((p) => minimatch(request.url, p))) {
console.log('SKIPPY MA-DOO-DAH',request.url);
Copy link
Member

Choose a reason for hiding this comment

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

lol

return false;
}
if (!compileMimeTypes.includes(getContentType(response))) {
return false;
}
Expand Down
7 changes: 6 additions & 1 deletion packages/polyserve/src/start_server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export interface ServerOptions {
/** Whether or not to compile JavaScript **/
compile?: 'always'|'never'|'auto';

/** Disable compilation for these filename patterns **/
doNotCompile?: string[];

/** Resolution algorithm to use for rewriting module specifiers */
moduleResolution?: 'none'|'node';

Expand Down Expand Up @@ -118,6 +121,7 @@ function applyDefaultServerOptions(options: ServerOptions) {
hostname: options.hostname || 'localhost',
root: path.resolve(options.root || '.'),
compile: options.compile || 'auto',
doNotCompile: options.doNotCompile || [],
certPath: options.certPath || 'cert.pem',
keyPath: options.keyPath || 'key.pem',
componentDir: getComponentDir(options),
Expand Down Expand Up @@ -401,7 +405,8 @@ export function getApp(options: ServerOptions): express.Express {
root,
options.packageName,
options.componentUrl,
options.componentDir));
options.componentDir,
options.doNotCompile));


app.use(`/${componentUrl}/`, polyserve);
Expand Down
1 change: 1 addition & 0 deletions packages/web-component-tester/runner/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export interface Config {
remote?: {};
origSuites?: string[];
compile?: 'auto'|'always'|'never';
doNotCompile?: string[];
skipCleanup?: boolean;
simpleOutput?: boolean;
skipUpdateCheck?: boolean;
Expand Down
1 change: 1 addition & 0 deletions packages/web-component-tester/runner/webserver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ Expected to find a ${mdFilenames.join(' or ')} at: ${pathToLocalWct}/
root: options.root,
componentDir,
compile: options.compile,
doNotCompile: options.doNotCompile,
hostname: options.webserver.hostname,
headers: DEFAULT_HEADERS,
packageName,
Expand Down
1 change: 1 addition & 0 deletions packages/web-component-tester/test/integration/browser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ function runsIntegrationSuite(
name: 'web-component-tester',
tags: ['org:Polymer', 'repo:web-component-tester'],
},
doNotCompile: ['/components/web-component-tester/browser.js'],
},
options, suiteOptions);
const context = new Context(allOptions);
Expand Down