feat: remove feature flag eszip #50
Conversation
github-actions
bot
added
the
type: feature
|
gotta fix some tests, but looks good generally :) |
|
@Skn0tt yep! Just spotted those, they didn't fail locally weirdly! On it 🙂 |
src/bundler.ts
Outdated
| @@ -65,21 +54,16 @@ const bundle = async ( | |||
| functions, | |||
| importMap, | |||
| }), | |||
| bundleESZIP({ | |||
There was a problem hiding this comment.
This means that we'll always generate two bundles (JS and ESZIP), which in the long term we don't want to do. What do you think about changing the logic so that:
- we use
trueas the default value foredge_functions_produce_eszip, so that it'll be enabled for CLI deployments - if the
edge_functions_produce_eszipflag is enabled, we generate only ESZIP - if the
edge_functions_produce_eszipflag is disabled, we generate only JS — this allows us to still flip a switch and go back to JS bundles in buildbot if something goes wrong
There was a problem hiding this comment.
Ohhhh gotcha, yeah makes sense to me, good shout!
There was a problem hiding this comment.
Fixed: da27e7a
|
(Also make sure to rename the PR to |
EwanValentine
changed the title
src/bundler.ts
Outdated
| await importMap.writeToFile(distImportMapPath) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This is to get around the statement limit per arrow function, hopefully, this isn't too arbitrarily grouped
There was a problem hiding this comment.
How do you feel about it? I would actually prefer to ignore the rule in this case, because it feels a bit arbitrary to break this bit out into a separate function just to satisfy the linter, and I think it actually makes the code a bit more difficult to follow.
There was a problem hiding this comment.
I definitely agree, wasn't sure how we felt about ignoring the linter though, but there didn't really seem to be a particularly clean split anywhere. I'll update to ignore it for this function
There was a problem hiding this comment.
Updated: 549e9fd
There was a problem hiding this comment.
Yeah, we should avoid ignoring rules generally. But in this case it does feel arbitrary, and we can perhaps discuss as a group whether we can make it a bit more lax.
* chore: remove eszip feature flag * chore: remove eszip feature flag * chore: fixed tests * chore: refactored to reduce statement limit * chore: disable max line limit eslint rule
🎉 Thanks for sending this pull request! 🎉
Which problem is this pull request solving?
Removes the use eszip feature flag.
Closes: #51
Checklist
Please add a
xinside each checkbox:A picture of a cute animal (not mandatory but encouraged)