Skip to content

Conversation

karolyi
Copy link
Contributor

@karolyi karolyi commented Dec 29, 2023

Season's greetings. :)

I need get_files() to be able to use skip_common_chunks just like render_bundle() does, so I took a couple hours and implemented it.

The goal here is to be able to render <link rel="prefetch"> (not preload) tags manually, skipping the ones from the chunk that are already downloaded via render_bundle().

Have this as my free contribution. :)

@karolyi
Copy link
Contributor Author

karolyi commented Jan 3, 2024

bump

used_urls = getattr(request, '_webpack_loader_used_urls', None)
if not used_urls:
used_urls = request._webpack_loader_used_urls = set()
return [x for x in result if x['url'] not in used_urls]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there's more code here, we need to test this. Can you add tests that covers all conditions here?

Copy link
Member

@fjsj fjsj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you contribution!
Overall looks good, but needs more tests to avoid a coverage decrease.

@karolyi
Copy link
Contributor Author

karolyi commented Jan 10, 2024

With all due respect, you are placing too much weight on the coverage. My code uses the same underlying functions that are already tested out.

Feel free to add tests though, I just can't be bothered.

@fjsj
Copy link
Member

fjsj commented Jan 10, 2024

Covering get_files with tests protects that code from future wrong changes too. This is necessary.
Will keep this open and I may have time to add this later this week, but no guarantee.

@fjsj fjsj merged commit 88bff84 into django-webpack:master Jan 15, 2024
@karolyi
Copy link
Contributor Author

karolyi commented Jan 15, 2024

Thanks for adding the tests. Could you please do a quick release so I can start using it?

@fjsj
Copy link
Member

fjsj commented Jan 16, 2024

3.0.1 published.

@karolyi
Copy link
Contributor Author

karolyi commented Jan 16, 2024

Thanks. Can confirm it's working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants