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.

Sign up for GitHub

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

Jump to bottom

WebP Transformer #4210

Closed
wants to merge 3 commits into from
Closed

WebP Transformer #4210

wants to merge 3 commits into from

Conversation

DeMoorJasper
Copy link
Member

@DeMoorJasper DeMoorJasper commented Feb 23, 2020 •
edited
Loading

↪️ Pull Request

Transformer images to the more optimised webp format using webp:...

Closes #3736

💻 Examples

import image from "webp:./image.jpg";

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

DeMoorJasper
DeMoorJasper commented Feb 23, 2020
View reviewed changes
packages/configs/default/index.json
@@ -26,6 +26,8 @@
"*.pug": ["@parcel/transformer-pug"],
"*.coffee": ["@parcel/transformer-coffeescript"],
"*.mdx": ["@parcel/transformer-mdx"],
"webp:*.{jpg,jpeg,png,tiff,webp}": ["@parcel/transformer-webp"],
"*.webp": ["@parcel/transformer-raw"],
Copy link
Member Author

@DeMoorJasper DeMoorJasper Feb 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It says transformer for ...webp not found without this line. Should fallback to raw shouldn't it? Or not try to find another pipeline

@parcel-benchmark
Copy link

parcel-benchmark commented Feb 23, 2020 •
edited
Loading

Benchmark Results

packages/benchmarks/kitchen-sink ✅

Timings

Description Time Difference
Cold 3.67s -7.00ms
Cached 3.44s +116.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.js 1.07kb +5.00b ⚠️ 202.00ms -12.00ms 🚀

Cached Bundles

Bundle Size Difference Time Difference
dist/index.js 1.07kb +5.00b ⚠️ 5.00ms -1.00ms 🚀

packages/benchmarks/react-hn ✅

Timings

Description Time Difference
Cold 8.55s -346.00ms
Cached 3.42s -154.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.js 67.47kb +429.00b ⚠️ 1.18s +4.00ms

Cached Bundles

Bundle Size Difference Time Difference
dist/index.js 67.47kb +429.00b ⚠️ 9.00ms +1.00ms ⚠️

packages/benchmarks/ak-editor ✅

Timings

Description Time Difference
Cold 4.82s +942.00ms ⚠️
Cached 3.36s -75.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.js 743.00b +7.00b ⚠️ 1.01s +849.00ms ⚠️

Cached Bundles

Bundle Size Difference Time Difference
dist/index.js 743.00b +7.00b ⚠️ 6.00ms +1.00ms ⚠️

Click here to view a detailed benchmark overview.

devongovett
devongovett reviewed Feb 24, 2020
View reviewed changes
packages/examples/kitchen-sink/src/index.js
@@ -1,5 +1,6 @@
import styles from './styles.css';
import parcel from 'url:./parcel.webp';
import parcelFront from 'webp:./parcel-front.png';
Copy link
Member

Choose a reason for hiding this comment

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

What ends up happening here? Is parcelFront a url? is the image inlined? realized we don't allow multiple named pipelines e.g. url:webp:./parcel-front.png, which seems like it might be problematic?

Copy link
Member Author

Choose a reason for hiding this comment

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

I mentioned it in my comment above, I had to add this to make it work at all "*.webp": ["@parcel/transformer-raw"]
I think it should return a url?
Although it currently returns an empty object which seems weird to me?

Not entirely sure if we should allow chaining although I don't really have a better idea.

@DeMoorJasper
Copy link
Member Author

Closing in favor of #4209

@DeMoorJasper DeMoorJasper deleted the webp-transformer branch February 27, 2020 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devongovett devongovett devongovett left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Parcel 2: WebP Transformer
3 participants
@DeMoorJasper @parcel-benchmark @devongovett