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

Clean old sourcemap assets #201

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Clean old sourcemap assets #201

merged 1 commit into from
Sep 4, 2024

Conversation

marcelolx
Copy link
Contributor

This PR #171 made propshaft to maintain sourcemap files, but they should be cleaned up the same way as normal assets are.

@@ -53,7 +53,7 @@ def all_files_from_tree(path)
end

def extract_path_and_digest(digested_path)
digest = digested_path.to_s[/-([0-9a-f]{7,128})\.(?!digested)[^.]+\z/, 1]
digest = digested_path.to_s[/-([0-9a-f]{7,128})\.(?!digested)/, 1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This regex should be the same as in server.rb:

digest = full_path[/-([0-9a-zA-Z]{7,128})\.(?!digested)([^.]|.map)+\z/, 1]

Might be worth defining this as a constant on Propshaft::Asset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason why the server one shouldn't be the same as this one? The PR that introduced support for multiple extensions after .digested has made the same change to the regex made here https://github.com/rails/propshaft/pull/14/files#diff-cea8115669d1b6f7ed0b417671e6b929c07d4f41b4e009b82182221f38b46955L45

I agree a constant might be worth it, so when you change it, all places use the new version

Copy link
Contributor Author

@marcelolx marcelolx Sep 4, 2024

Choose a reason for hiding this comment

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

Just not sure if we should use the same regex in all places (even though there a few different places are defining a regex like this )

rafaelfranca added a commit that referenced this pull request Sep 4, 2024
@rafaelfranca rafaelfranca merged commit 8750853 into rails:main Sep 4, 2024
3 checks passed
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.

3 participants