You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change allows preprocessors to specify an encoding for files. Currently, if a preprocessor encodes a file in a format such as gzip or Brotli, the file is served with the wrong encoding. The only workaround for this is to also include a plugin (example) that removes encoded files from files.served and serves them manually using a custom file handler. This approach is a gross hack that unnecessarily complicates the encoding plugin's logic.
With this change, a preprocessor encodes the contents and stores them in the File class' new encodings property. The encodings property is a keyed object where the keys are the encoding type ('gzip', 'compress', 'br', etc.) and the values are the encoded content. This pattern has the advantage of allowing multiple encoders and serving the encoding that best matches the request. It also allows the unencoded content to be served, when no encoding matches the request.
I know there has been pushback on adding encoding support, but I'm hoping this approach is an acceptable compromise. This solution leaves pretty much all encoding logic to the preprocessors and minimizes karma's role as much as possible.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The reason will be displayed to describe this comment to others. Learn more.
You mentioned pushback, can you point to it so I can understand the issue. Your approach looks good to me.
Can you add a short bit of documentation to dev/plugins.html under preprocessors?
I'm thinking of a slight re-write of your commit description:
a preprocessor encodes the contents and stores them in the File class' new encodings property. The encodings property is a keyed object where the keys are the encoding type ('gzip', 'compress', 'br', etc.) and the values are the encoded content.
Can you add a short bit of documentation to dev/plugins.html under preprocessors?
I'm thinking of a slight re-write of your commit description:
Done. It felt odd to document only the encodings property of the file object, so I added documentation for all three arguments passed to preprocessors. For the file argument, I documented the path, originalPath, encodings, and type properties. I did not document contentPath, mtime, isUrl, and doNotCache; because they seemed more internal to karma and not really related to preprocessors. Let me know if you'd like me to document any of the properties I did not document, or if you'd like me to remove any of the properties I did document.
I think your feature is more general than the feature request that was rejected earlier. It fits into karma's architecture, allowing other plugins without a large impact on the core code.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GreenGremlin
force-pushed
the
source-file-encoding
branch
2 times, most recently
from
ghost
mentioned this pull request
This change allows preprocessors to specify an encoding for files. Currently, if a preprocessor encodes a file in a format such as gzip or Brotli, the file is served with the wrong encoding. The only workaround for this is to also include a plugin (example) that removes encoded files from
files.servedand serves them manually using a custom file handler. This approach is a gross hack that unnecessarily complicates the encoding plugin's logic.With this change, a preprocessor encodes the contents and stores them in the File class' new
encodingsproperty. Theencodingsproperty is a keyed object where the keys are the encoding type ('gzip', 'compress', 'br', etc.) and the values are the encoded content. This pattern has the advantage of allowing multiple encoders and serving the encoding that best matches the request. It also allows the unencoded content to be served, when no encoding matches the request.I know there has been pushback on adding encoding support, but I'm hoping this approach is an acceptable compromise. This solution leaves pretty much all encoding logic to the preprocessors and minimizes karma's role as much as possible.