Skip to content
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

Warn and fix missing mime types #1050

Merged
merged 18 commits into from
Jun 15, 2023

Conversation

Archmonger
Copy link
Contributor

@Archmonger Archmonger commented Jun 14, 2023

Issues

N/A

Summary

Windows registry can sometimes go corrupt causing missing mime types. This results in our backends assuming all file types are text/plain. Ultimately, this causes us to fail to serve our ReactPy client index.js.

This PR aims to only resolve a minimal set of file extensions (only the ones we potentially use within ReactPy core). I considered using a more comprehensive mime_types list from this repo, but it seems a bit overkill for us right now. We may want to use it if we need to serve a variety of static files.

Checklist

  • Tests have been included for all bug fixes or added functionality.
  • The changelog.rst has been updated with any significant changes.

@Archmonger Archmonger marked this pull request as ready for review June 14, 2023 05:24
@Archmonger Archmonger requested a review from rmorshea June 14, 2023 05:25
@rmorshea
Copy link
Collaborator

I'll review this evening.

Copy link
Collaborator

@rmorshea rmorshea left a comment

Choose a reason for hiding this comment

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

I wonder if we should be really trying to patch this or just warning about it and linking to resources on the Windows mimetype registry - neither Starlette nor Flask appear to make any similar attempt.

src/py/reactpy/reactpy/backend/__init__.py Outdated Show resolved Hide resolved
@Archmonger
Copy link
Contributor Author

Archmonger commented Jun 15, 2023

I thought the same thing at first.

But the patch is a harmless fix for reactpy.run, which is a user's first impression of our framework. If we can improve first impressions with minimal LOCs it's in our best interest to do so.

I didn't want to link to anything specific since the repair method depends on operating system. This issue technically isn't limited to Windows, but it is far more likely to happen on Windows.

I will update the comment to be more specific about this issue being OS independent.

@rmorshea
Copy link
Collaborator

I just merged a fix for the test-docs failure.

@rmorshea rmorshea added this to the 1.0.1 milestone Jun 15, 2023
Co-authored-by: Ryan Morshead <ryan.morshead@gmail.com>
@rmorshea
Copy link
Collaborator

After that last comment LGTM

rmorshea
rmorshea previously approved these changes Jun 15, 2023
@Archmonger Archmonger changed the title Fix Windows mime types bug Warn and fix missing mime types Jun 15, 2023
@Archmonger Archmonger merged commit bd60e6e into reactive-python:main Jun 15, 2023
17 checks passed
@Archmonger Archmonger deleted the fix-mime-type branch June 15, 2023 06:15
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.

None yet

2 participants