Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Adding skip_common_chunks option to the get_files() template tag #386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fjsj merged 5 commits into django-webpack:master from karolyi:master
Jan 15, 2024
Merged

Adding skip_common_chunks option to the get_files() template tag #386

fjsj merged 5 commits into django-webpack:master from karolyi:master
Jan 15, 2024

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. :)

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
edited
Loading

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.

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.

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.

karolyi reacted with thumbs up emoji

@fjsj fjsj merged commit 88bff84 into django-webpack:master Jan 15, 2024
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?

Copy link
Member

fjsj commented Jan 16, 2024

3.0.1 published.

Copy link
Contributor Author

karolyi commented Jan 16, 2024

Thanks. Can confirm it's working.

fjsj reacted with hooray emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fjsj fjsj fjsj approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /