refactor: use the fetch API#33
Conversation
Should drop a single microtask. Doesn't really make a difference in practice, but it doesn't hurt either.
wolfenrain
left a comment
wolfenrain
left a comment
There was a problem hiding this comment.
LGTM! This is a lot clearer in what it does.
| reject(new Error(`Failed to load: ${req.status} ${req.statusText}`)); | ||
| } | ||
| }; | ||
| function reportProgress() { |
There was a problem hiding this comment.
This is out of the scope for the refactor but I wonder if we should write some tests for this loader to just verify it works as intended
There was a problem hiding this comment.
Yes, the initial effort didn't have any tests. I would very much like to have some tests on the template output that verify the loading behavior is as expected; although it might be a bit complex to assess such.
|
Oh, another thing is that this is in sloppy mode, probably worth adding a |
I am not against adding that here directly, afaik it is just a one-liner right? |
This could theoretically change behavior for `{{flutter_js}}`, but I think it's fine.
4 participants
Status
READY
Description
Rewrote it all! :)
In more seriousness:
Probably review one-commit-at-a-time?
Closes: #32
Type of Change