Skip to content

Commit

Permalink
remove polyfill-node / polyfill_node
Browse files Browse the repository at this point in the history
The quality of the polyfills is quite bad, so I'm removing this implementation (introduced in  #72)
  • Loading branch information
threepointone committed Dec 13, 2021
1 parent 7858ca2 commit 219f654
Show file tree
Hide file tree
Showing 7 changed files with 5 additions and 71 deletions.
4 changes: 1 addition & 3 deletions packages/wrangler/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,7 @@
"esbuild": "0.14.1",
"miniflare": "2.0.0-rc.3",
"semiver": "^1.1.0",
"serve": "^13.0.2",
"@esbuild-plugins/node-globals-polyfill": "^0.1.1",
"@esbuild-plugins/node-modules-polyfill": "^0.1.2"
"serve": "^13.0.2"
},
"optionalDependencies": {
"fsevents": "~2.3.2"
Expand Down
9 changes: 1 addition & 8 deletions packages/wrangler/scripts/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,7 @@ async function run() {
platform: "node",
format: "cjs",
// minify: true, // TODO: enable this again
external: [
"fsevents",
"esbuild",
"miniflare",
"@miniflare/core",
"@esbuild-plugins/node-globals-polyfill",
"@esbuild-plugins/node-modules-polyfill",
], // todo - bundle miniflare too
external: ["fsevents", "esbuild", "miniflare", "@miniflare/core"], // todo - bundle miniflare too
sourcemap: true,
inject: [path.join(__dirname, "../import_meta_url.js")],
define: {
Expand Down
2 changes: 0 additions & 2 deletions packages/wrangler/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ type Env = {
site?: Site;
jsx_factory?: string; // inherited
jsx_fragment?: string; // inherited
polyfill_node?: boolean; //inherited
// we should use typescript to parse cron patterns
triggers?: { crons: Cron[] }; // inherited
vars?: Vars;
Expand All @@ -108,7 +107,6 @@ export type Config = {
webpack_config?: string; // inherited
jsx_factory?: string; // inherited
jsx_fragment?: string; // inherited
polyfill_node?: boolean; //inherited
vars?: Vars;
migrations?: DOMigration[];
durable_objects?: { bindings: DurableObject[] };
Expand Down
22 changes: 2 additions & 20 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import commandExists from "command-exists";
import assert from "assert";
import { getAPIToken } from "./user";
import fetch from "node-fetch";
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";

type CfScriptFormat = void | "modules" | "service-worker";

Expand All @@ -42,7 +40,6 @@ type Props = {
compatibilityDate: void | string;
compatibilityFlags: void | string[];
usageModel: void | "bundled" | "unbound";
polyfillNode: void | boolean;
};

export function Dev(props: Props): JSX.Element {
Expand All @@ -61,7 +58,6 @@ export function Dev(props: Props): JSX.Element {
staticRoot: props.public,
jsxFactory: props.jsxFactory,
jsxFragment: props.jsxFragment,
polyfillNode: props.polyfillNode,
});
if (bundle && bundle.type === "commonjs" && !props.format && props.public) {
throw new Error(
Expand Down Expand Up @@ -355,26 +351,15 @@ function useEsbuild(props: {
staticRoot: void | string;
jsxFactory: string | void;
jsxFragment: string | void;
polyfillNode: boolean | void;
}): EsbuildBundle | void {
const {
entry,
destination,
staticRoot,
jsxFactory,
jsxFragment,
polyfillNode,
} = props;
const { entry, destination, staticRoot, jsxFactory, jsxFragment } = props;
const [bundle, setBundle] = useState<EsbuildBundle>();
useEffect(() => {
let result: esbuild.BuildResult;
async function build() {
if (!destination) return;
result = await esbuild.build({
entryPoints: [entry],
define: {
...(polyfillNode && { global: "globalThis" }),
},
bundle: true,
outdir: destination,
metafile: true,
Expand All @@ -383,9 +368,6 @@ function useEsbuild(props: {
loader: {
".js": "jsx",
},
plugins: polyfillNode
? [NodeGlobalsPolyfills({ buffer: true }), NodeModulesPolyfills()]
: undefined,
...(jsxFactory && { jsxFactory }),
...(jsxFragment && { jsxFragment }),
external: ["__STATIC_CONTENT_MANIFEST"],
Expand Down Expand Up @@ -418,7 +400,7 @@ function useEsbuild(props: {
return () => {
result?.stop();
};
}, [entry, destination, staticRoot, jsxFactory, jsxFragment, polyfillNode]);
}, [entry, destination, staticRoot, jsxFactory, jsxFragment]);
return bundle;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/dialogs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type PromptProps = {
onSubmit: (text: string) => void;
};

export function Prompt(props: PromptProps) {
function Prompt(props: PromptProps) {
const [value, setValue] = useState("");

return (
Expand Down
25 changes: 0 additions & 25 deletions packages/wrangler/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ async function readConfig(path?: string): Promise<Config> {
"route",
"jsx_factory",
"jsx_fragment",
"polyfill_node",
"site",
"triggers",
"usage_model",
Expand Down Expand Up @@ -422,10 +421,6 @@ export async function main(argv: string[]): Promise<void> {
.option("jsx-fragment", {
describe: "The function that is called for each JSX fragment",
type: "string",
})
.option("polyfill-node", {
describe: "Try to polyfill node builtins",
type: "boolean",
});
},
async (args) => {
Expand Down Expand Up @@ -454,13 +449,6 @@ export async function main(argv: string[]): Promise<void> {

const envRootObj = args.env ? config.env[args.env] || {} : config;

const polyfillNode = args["polyfill-node"] ?? config.polyfill_node;
if (polyfillNode) {
console.warn(
"Using polyfills for node builtin modules. These are not meant to be completely reliable, and have serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details."
);
}

// TODO: this error shouldn't actually happen,
// but we haven't fixed it internally yet
if ("durable_objects" in envRootObj) {
Expand All @@ -483,7 +471,6 @@ export async function main(argv: string[]): Promise<void> {
accountId={config.account_id}
site={args.site || config.site?.bucket}
port={args.port || config.dev?.port}
polyfillNode={polyfillNode}
public={args.public}
compatibilityDate={config.compatibility_date}
compatibilityFlags={config.compatibility_flags}
Expand Down Expand Up @@ -564,10 +551,6 @@ export async function main(argv: string[]): Promise<void> {
.option("jsx-fragment", {
describe: "The function that is called for each JSX fragment",
type: "string",
})
.option("polyfill-node", {
describe: "Try to polyfill node builtins",
type: "boolean",
});
},
async (args) => {
Expand Down Expand Up @@ -596,13 +579,6 @@ export async function main(argv: string[]): Promise<void> {

// -- snip, end --

const polyfillNode = args["polyfill-node"] ?? config.polyfill_node;
if (polyfillNode) {
console.warn(
"Using polyfills for node builtin modules. These are not meant to be completely reliable, and have serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details."
);
}

await publish({
config: args.config as Config,
name: args.name,
Expand All @@ -611,7 +587,6 @@ export async function main(argv: string[]): Promise<void> {
triggers: args.triggers,
jsxFactory: args["jsx-factory"],
jsxFragment: args["jsx-fragment"],
polyfillNode: args["polyfill-node"],
routes: args.routes,
public: args.public,
site: args.site,
Expand Down
12 changes: 0 additions & 12 deletions packages/wrangler/src/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import { readFile } from "fs/promises";
import cfetch from "./cfetch";
import assert from "node:assert";
import { syncAssets } from "./sites";
import NodeModulesPolyfills from "@esbuild-plugins/node-modules-polyfill";
import NodeGlobalsPolyfills from "@esbuild-plugins/node-globals-polyfill";

type CfScriptFormat = void | "modules" | "service-worker";

Expand All @@ -26,7 +24,6 @@ type Props = {
legacyEnv?: boolean;
jsxFactory: void | string;
jsxFragment: void | string;
polyfillNode: void | boolean;
};

function sleep(ms: number) {
Expand Down Expand Up @@ -94,19 +91,10 @@ export default async function publish(props: Props): Promise<void> {
}
: { entryPoints: [file] }),
bundle: true,
define: {
...((props.polyfillNode ?? config.polyfill_node) && {
global: "globalThis",
}),
},
nodePaths: props.public ? [path.join(__dirname, "../vendor")] : undefined,
outdir: destination.path,
external: ["__STATIC_CONTENT_MANIFEST"],
format: "esm",
plugins:
props.polyfillNode ?? config.polyfill_node
? [NodeGlobalsPolyfills({ buffer: true }), NodeModulesPolyfills()]
: undefined,
sourcemap: true,
metafile: true,
loader: {
Expand Down

0 comments on commit 219f654

Please sign in to comment.