Conversation
vicb
left a comment
There was a problem hiding this comment.
src/workerd/api/tests/opennextjs/README.md is gigantic :(
One way to simplify would be to use the migrate command created by @dario-piotrowicz recently.
Maybe something simpler would be to add an exemple in the ON repo and use that? I doubt the readme would be maintained here?
|
186k+ lines are nearly impossible to adequately review. Is there a way to have much of this automated / generated dynamically by bazel at build time to cut down on the static file size, particular in the worker file? |
22d3139 to
662ba49
Compare
662ba49 to
560b53d
Compare
|
@jasnell I've force-pushed and address all concerns. The changes are much more readable/reviewable now. |
3eb83e4 to
3e0019f
Compare
e8303a9 to
afa1db9
Compare
afa1db9 to
0028776
Compare
fhanau
left a comment
There was a problem hiding this comment.
I think that's all of the concerns I had about the build changes – LGTM but someone familiar with the test changes should approve.
vicb
left a comment
There was a problem hiding this comment.
LGTM for the Next app - not familiar enough with bazel to review that part.
I added minor comments about idiomatic Next and flags
0028776 to
ff7d244
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
ff7d244 to
4270776
Compare
4270776 to
6854c87
Compare
Related Task: EW-10513
Added a task with the help of AI to write some tests using a real opennextjs bundle. Added a REAADME for generating them later on. PS: I've trimmed as much as I can from the output that isn't important to us (in this context).