Skip to content

Commit

Permalink
code review
Browse files Browse the repository at this point in the history
  • Loading branch information
Cass Fridkin committed Apr 14, 2022
1 parent 5e1e148 commit 84893e1
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 54 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pullrequests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
node-version: 16.7
cache: "npm"

- name: Install Wrangler 1
- name: Restore Wrangler 1 from Cache
uses: actions/cache@v3
id: wrangler-1-cache
with:
Expand Down
2 changes: 1 addition & 1 deletion docs/eject-webpack.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ module.exports = {
}
```

3. Remove unsupported entires from your `wrangler.toml`
3. Remove unsupported entries from your `wrangler.toml`

Remove the `type` and `webpack_config` keys from your `wrangler.toml`, as they're not supported anymore.

Expand Down
7 changes: 2 additions & 5 deletions packages/wranglerjs-compat-webpack-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"main": "lib/index.js",
"types": "lib/index.d.ts",
"bugs": {
"url": "https://github.com/cloudflare/wrangler/issues"
"url": "https://github.com/cloudflare/wrangler2/issues"
},
"homepage": "https://github.com/cloudflare/wrangler#readme",
"homepage": "https://github.com/cloudflare/wrangler2#readme",
"scripts": {
"build": "run-p build:*",
"build:d.ts": "tsc",
Expand Down Expand Up @@ -39,9 +39,6 @@
"webpack": "^4.46.0",
"wrangler": "../wrangler"
},
"peerDependencies": {
"webpack": "^4.46.0"
},
"jest": {
"restoreMocks": true,
"testTimeout": 180000,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ export const cleanMessage = (message: string): string =>
message
.replaceAll(/^.*debugger.*$/gim, "") // remove debugger statements
.replaceAll(/\d+ms/gm, "[timing]") // standardize timings
.replaceAll(process.cwd(), "[temp dir]") // standardize directories
.replaceAll(process.cwd(), "[dir]") // standardize directories
.replaceAll(process.cwd().replaceAll("\\", "/"), "[dir]")
.replaceAll(PATH_TO_WRANGLER, "[wrangler 1]") // standardize calls to wrangler 1
.replaceAll(/found .+ vulnerabilities/gm, "found [some] vulnerabilities") // vuln counts
.replaceAll(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export function writeWebpackConfig(
let stringified = "";

if (usePlugin) {
// stringified += `debugger;\n`;
stringified += `const { WranglerJsCompatWebpackPlugin } = require("wranglerjs-compat-webpack-plugin");\n`;

config.plugins = config.plugins || [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { WranglerJsCompatWebpackPlugin } from "../";
import { compareOutputs } from "./helpers/compare-outputs";
import { installWrangler1 } from "./helpers/install-wrangler";
import { mockConfigDir } from "./helpers/mock-config-dir";
import { cleanMessage } from "./helpers/pipe";

mockAccountId();
mockApiToken();
Expand All @@ -36,13 +37,9 @@ describe("messaging", () => {

await expect(runWebpack(config)).resolves.not.toThrow();

expect(std.out).toMatchInlineSnapshot(`""`);
expect(std.err).toMatchInlineSnapshot(`""`);
expect(
std.warn
.replaceAll(process.cwd(), "[dir]")
.replaceAll(process.cwd().replaceAll("\\", "/"), "[dir]")
).toMatchInlineSnapshot(`
expect(cleanMessage(std.out)).toMatchInlineSnapshot(`""`);
expect(cleanMessage(std.err)).toMatchInlineSnapshot(`""`);
expect(cleanMessage(std.warn)).toMatchInlineSnapshot(`
"Setting \`target\` to \\"webworker\\"...
Running \`npm install\` in [dir]..."
`);
Expand Down Expand Up @@ -71,47 +68,46 @@ describe("wrangler 1 parity", () => {
expect(wrangler2.result).not.toBeInstanceOf(Error);

expect(wrangler1.output).toStrictEqual(wrangler2.output);
});

// TODO
it("works with webassembly", async () => {
return;
const { wrangler1, wrangler2 } = await compareOutputs({
webpackConfig: {
entry: "./index.js",
target: "webworker",
},
wranglerConfig: {
main: "./worker/script.js",
},
worker: { type: "sw" },
});

expect(wrangler1.result).not.toBeInstanceOf(Error);
expect(wrangler2.result).not.toBeInstanceOf(Error);

expect(wrangler1.output).toStrictEqual(wrangler2.output);
expect(wrangler1.std.out).toMatchInlineSnapshot(`
"up to date, audited 1 package in [timing]
found [some] vulnerabilities
Built successfully, built project size is 503 bytes."
`);
expect(wrangler1.std.err).toMatchInlineSnapshot(`""`);
expect(wrangler1.std.warn).toMatchInlineSnapshot(`""`);

expect(wrangler2.std.out).toMatchInlineSnapshot(`
"running: npm run build
> build
> webpack --no-color
Hash: e96932fc5c1ce19ddd05
Version: webpack 4.46.0
Time: [timing]
Built at: [time]
Asset Size Chunks Chunk Names
main.js 1020 bytes 0 main
Entrypoint main = main.js
[0] ./index.js + 1 modules 163 bytes {0} [built]
| ./index.js 140 bytes [built]
| ./another.js 23 bytes [built]
Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
test-name.test-sub-domain.workers.dev"
`);
expect(wrangler2.std.err).toMatchInlineSnapshot(`""`);
expect(wrangler2.std.warn).toMatchInlineSnapshot(`
"WARNING in configuration
The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/configuration/mode/"
`);
});

// TODO
it("works with sites", async () => {
return;
const { wrangler1, wrangler2 } = await compareOutputs({
webpackConfig: {
entry: "./index.js",
target: "webworker",
},
wranglerConfig: {
main: "./worker/script.js",
},
worker: { type: "sw" },
});

expect(wrangler1.result).not.toBeInstanceOf(Error);
expect(wrangler2.result).not.toBeInstanceOf(Error);
it.todo("works with webassembly");

expect(wrangler1.output).toStrictEqual(wrangler2.output);
});
it.todo("works with sites");
});

async function runWebpack(
Expand Down
2 changes: 1 addition & 1 deletion packages/wranglerjs-compat-webpack-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export class WranglerJsCompatWebpackPlugin {
return;
}

const warning = `wrangler 1 would automatically clone a simple worker into ${this.packageDir} for you if it detected a sites project with no worker component, but you should now include the worker yourself.`;
const warning = `We're going to clone a simple worker into ${this.packageDir} for you since we detected a sites project with no worker component. To hide this warning, you should include the worker before building.`;
const heresTheTemplate = `You can clone the worker into ${this.packageDir} yourself:`;
const template = "https://github.com/cloudflare/worker-sites-init";

Expand Down

0 comments on commit 84893e1

Please sign in to comment.