JSON files included in the worker script

[rubenamorim]

🐛 Bug Report

JSON files are included in the worker script processed by the webpack.
This occurs in the filterByExtension function in wranglerjs because it evaluates to true with the js extension passed as an argument and matches for js and json files.

Environment

• operating system:
• output of rustc -V: 1.49.0
• output of node -v: 14.15
• output of wrangler -V: 1.12.3
• contents of wrangler.toml

account_id = "$CF_ACCOUNT_ID"
name = "example"
route = ""
type = "webpack"
webpack_config = "node_modules/flareact/webpack"
workers_dev = true
zone_id = ""

[site]
bucket = "out"
entry-point = "./"

Steps to reproduce

1. Create a worker site using the Flareact template: wrangler generate my-project https://github.com/flareact/flareact-template
2. Add a json file to the public folder to be served as a static asset
3. Build and deploy: yarn deploy

What did you expect to see?

The content of the JSON file is not included in the worker's script file.

What did you see instead?

The worker script file includes the content of the JSON file.

EDIT:

Here's an incorrectly generated script file and the attempt to publish it:

[xtuc]

I'm not sure to understand how your PR is fixing the issue. Could you please share the invalid generated code (privately by email if you need)?

[rubenamorim]

@xtuc My PR just fixes the regex that catches json files as a js asset and generates an invalid worker script.

Here's the repo with the example. The produced worker script is here and you can see a JSON file included in the worker script (line 23) because it is included in the bucket folder to be served as a static asset.

Thanks

[xtuc]

I understand the issue now, thanks. I think just emiting a JSON file won't work, the worker still has to get access to it something.

Either you can ask webpack to inline it, which will work, or wrangler needs to upload it.
We recently added #1543, we could upload the JSON file as a text binding and rewrite the JS to point to that global JS value.

[rubenamorim]

Is it okay to filter for JS files here and get JSON files to add to the script without import them?

Thanks but text blobs don't meet our requirements because we can put a manifest.json file in a KV to serve it onworker-domain.com/manifest.json using only the getAssetFromKV magic 😕

[xtuc]

Is it okay to filter for JS files here and get JSON files to add to the script without import them?

The filtering make sense to me. However we need to import the file to be able to use it in the worker.

I don't think we should require KV. Text blobs are files attached to the worker, similar to injecting them inline. It looks like it's undocumented right now.
One downside of that approach is that since the binding is injected as a string the json string need to be parsed against to be the original object.

I suggest we wait for #1677 which will allow to import JSON files and likely will upload them.

[rubenamorim]

#1677 Looks great for the issue, we will wait. Thanks 🙏

Feel free to close my PR and the issue if you want 😉

[xtuc]

We can keep it open. As a workaround for now, you can ask webpack to inline the JSON into your script, which will avoid the buggy codepath in wrangler(js).