Don't discard invalidations when transformer throws an error #7547

mischnic · 2022-01-10T21:31:18Z

No invalidation gets added to the graph if runTransform throws an error. So instead catch it and return the error along with invalidations that were added along the way

Closes #6124

height · 2022-01-10T21:31:20Z

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

parcel-benchmark · 2022-01-10T22:17:09Z

Benchmark Results

Kitchen Sink ✅

Timings

Description	Time	Difference
Cold	2.36s	+118.00ms
Cached	371.00ms	+40.00ms ⚠️

Cold Bundles

Bundle	Size	Difference	Time	Difference
dist/legacy/parcel.7cdb0fad.webp	102.94kb	+0.00b	105.00ms	-551.00ms 🚀
dist/legacy/parcel.7cdb0fad.webp	102.94kb	+0.00b	710.00ms	+54.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp	102.94kb	+0.00b	710.00ms	+54.00ms ⚠️

Cached Bundles

Bundle	Size	Difference	Time	Difference
dist/legacy/index.html	826.00b	+0.00b	1.13s	-90.00ms 🚀
dist/modern/index.html	749.00b	+0.00b	1.13s	-92.00ms 🚀
dist/legacy/index.c1bc86aa.css	94.00b	+0.00b	1.27s	-140.00ms 🚀
dist/modern/index.57a95cbe.css	94.00b	+0.00b	1.27s	-139.00ms 🚀

React HackerNews ✅

Timings

Description	Time	Difference
Cold	10.69s	-242.00ms
Cached	467.00ms	-12.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

AtlasKit Editor ✅

Timings

Description	Time	Difference
Cold	1.19m	+195.00ms
Cached	1.54s	+2.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle	Size	Difference	Time	Difference
dist/editorView.af442747.js	594.92kb	+0.00b	53.73s	+13.85s ⚠️
dist/popup.64bc9a82.js	209.67kb	+0.00b	53.73s	+13.85s ⚠️
dist/Toolbar.1af0e801.js	107.23kb	+0.00b	53.73s	+13.85s ⚠️
dist/Modal.cd71eaf3.js	45.33kb	+0.00b	53.73s	+13.85s ⚠️
dist/ui.5d3f7adc.js	14.94kb	+0.00b	53.74s	+13.85s ⚠️
dist/smartMediaEditor.48c8cf63.js	13.25kb	+0.00b	53.73s	+13.85s ⚠️
dist/dropzone.39132d0c.js	12.16kb	+0.00b	53.73s	+13.84s ⚠️
dist/EmojiPickerComponent.0482d6c0.js	3.73kb	+0.00b	53.73s	+13.85s ⚠️
dist/dropzone.55bef257.js	3.29kb	+0.00b	53.73s	+13.85s ⚠️
dist/clipboard.df70240c.js	2.93kb	+0.00b	53.73s	+13.85s ⚠️
dist/ResourcedEmojiComponent.667554b4.js	2.12kb	+0.00b	53.74s	+13.85s ⚠️
dist/browser.4e039ed7.js	1.69kb	+0.00b	53.73s	+13.85s ⚠️
dist/media-card-analytics-error-boundary.c718a9a7.js	1.12kb	+0.00b	53.73s	+13.85s ⚠️
dist/media-picker-analytics-error-boundary.8b2547e5.js	966.00b	+0.00b	53.73s	+13.85s ⚠️
dist/simpleHasher.fc0d6100.js	643.00b	+0.00b	53.73s	+13.85s ⚠️

Three.js ✅

Timings

Description	Time	Difference
Cold	7.43s	+4.00ms
Cached	406.00ms	+18.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

mischnic · 2022-01-10T22:43:17Z

Somehow returning the ThrowableDiagnostic object as opposed to throwing it strips the diagnostics property on the other thread. Might be a difference between structured cloning vs v8's serialization...

mischnic · 2022-01-12T22:19:25Z

Turns out that when throwing an error the worker farm automatically serializes

parcel/packages/core/workers/src/WorkerFarm.js

Line 309 in 4745cd3

content: anyToDiagnostic(e),

and deserializes

parcel/packages/core/workers/src/WorkerFarm.js

Lines 337 to 339 in 4745cd3

    
           if (result.contentType === 'error') { 
        
             throw new ThrowableDiagnostic({diagnostic: result.content}); 
        
           }

the ThrowableDiagnostic. So I'm doing that manually now for that added error property on the return value.

objarni · 2022-01-16T21:23:28Z

Would love to test this out. Anyway I can help?

packages/core/integration-tests/test/cache.js

parcel-benchmark · 2022-01-16T23:13:29Z

Benchmark Results

Kitchen Sink ✅

Timings

Description	Time	Difference
Cold	1.79s	-0.00ms
Cached	291.00ms	-0.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

React HackerNews ✅

Timings

Description	Time	Difference
Cold	8.98s	+82.00ms
Cached	416.00ms	-15.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle	Size	Difference	Time	Difference
dist/logo.c5bb83f1.png	246.00b	+0.00b	4.09s	-584.00ms 🚀

AtlasKit Editor ✅

Timings

Description	Time	Difference
Cold	58.34s	-479.00ms
Cached	1.40s	+33.00ms

Cold Bundles

Bundle	Size	Difference	Time	Difference
dist/editorView.af442747.js	594.92kb	+0.00b	32.49s	-11.63s 🚀
dist/popup.64bc9a82.js	209.67kb	+0.00b	32.48s	-11.64s 🚀
dist/Toolbar.1af0e801.js	107.23kb	+0.00b	32.49s	-11.63s 🚀
dist/Modal.cd71eaf3.js	45.33kb	+0.00b	32.48s	-11.64s 🚀
dist/ui.5d3f7adc.js	14.94kb	+0.00b	32.49s	-11.64s 🚀
dist/smartMediaEditor.48c8cf63.js	13.25kb	+0.00b	32.48s	-11.64s 🚀
dist/dropzone.39132d0c.js	12.16kb	+0.00b	32.48s	-11.64s 🚀
dist/EmojiPickerComponent.0482d6c0.js	3.73kb	+0.00b	32.49s	-11.64s 🚀
dist/dropzone.55bef257.js	3.29kb	+0.00b	32.49s	-11.63s 🚀
dist/clipboard.df70240c.js	2.93kb	+0.00b	32.48s	-11.64s 🚀
dist/ResourcedEmojiComponent.667554b4.js	2.12kb	+0.00b	32.49s	-11.64s 🚀
dist/browser.4e039ed7.js	1.69kb	+0.00b	32.48s	-11.64s 🚀
dist/media-card-analytics-error-boundary.c718a9a7.js	1.12kb	+0.00b	32.48s	-11.64s 🚀
dist/media-picker-analytics-error-boundary.8b2547e5.js	966.00b	+0.00b	32.49s	-11.64s 🚀
dist/simpleHasher.fc0d6100.js	643.00b	+0.00b	32.49s	-11.64s 🚀

Cached Bundles

No bundle changes detected.

Three.js ✅

Timings

Description	Time	Difference
Cold	6.27s	-3.00ms
Cached	341.00ms	-19.00ms 🚀

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle	Size	Difference	Time	Difference
dist/Three.js	579.68kb	+0.00b	4.64s	-427.00ms 🚀

Click here to view a detailed benchmark overview.

n1k0 · 2022-01-17T22:05:00Z

Great, thank you so much folks <3 Do we have an idea when this will be released?

Edit: Sorry, I just realized it shipped along v2.2.1 already. And… it solved the very last issue I had with Parcel v2 🎉

You're amazing, keep up the good work 👍 🙏

parcel-bundler/parcel#7547 🎉

objarni · 2022-01-18T19:38:14Z

Thanks for your efforts!

Did some testing with parcel 2.2.1 and a small Elm app (https://github.com/objarni/KataDB/ ) with following setup:

npm install
npm start

npm start is a shorthand for "parcel".

Observations using Sublime Text's Ctrl+S to save and Firefox browser:

Modifying title in src/index.html hotreloads
Changing string "Filter by tag" in src/Main.elm hotreloads
Changing string "Most recently used" in src/KataDb.elm DOES NOT hotreload, however hitting Ctrl+R in Firefox does refresh the page/app, so rebuilding of the Elm app has probably happened "behind the scenes" but not percolated to the surface. Edit: indeed rebuilding does happen, I can see the terminal "Built in 66ms" flashing "Building" when saving KataDb.elm!

Hypothesis: hotreloading works for html files and the "first" Elm file (Main.elm), but not dependent .elm files like KataDb.

w0rm · 2022-01-19T07:59:56Z

@objarni I imagine hot reloading would keep the current model and swap out update and view. KataDb.elm contains a constant that is used to initiate the model, therefore changing it doesn't hot reload at runtime.

objarni · 2022-01-19T09:02:27Z

@w0rm that makes sense! Do you have any advice on what would trigger a re-init of the model except refreshing page?

w0rm · 2022-01-19T12:51:50Z

@objarni I think manually refreshing the page is probably acceptable in such cases. It is a non trivial task for the hot reload to tell apart this type of change.

mischnic requested a review from devongovett January 10, 2022 21:31

mischnic added 3 commits January 12, 2022 22:06

Add test

8d8161b

Catch runPipelines error to retain invalidations

1dd65a7

Correctly serialize ThrowableDiagnostic from transformers

d9a55b7

mischnic force-pushed the transformer-error-invalidations branch from 7e4572f to d9a55b7 Compare January 12, 2022 22:19

mischnic mentioned this pull request Jan 14, 2022

Hotreloading for Elm submodules doesn't work after compilation error #7557

Closed

mischnic commented Jan 16, 2022

View reviewed changes

packages/core/integration-tests/test/cache.js Outdated Show resolved Hide resolved

Cleanup

df58521

devongovett approved these changes Jan 17, 2022

View reviewed changes

devongovett merged commit 6094369 into v2 Jan 17, 2022

devongovett deleted the transformer-error-invalidations branch January 17, 2022 16:40

n1k0 added a commit to MTES-MCT/ecobalyse that referenced this pull request Jan 17, 2022

Upgrade Parcel to v2.2.1, removed Elm hot-reload hacks.

d34bb54

parcel-bundler/parcel#7547 🎉

n1k0 mentioned this pull request Jan 17, 2022

Upgrade Parcel to v2.2.1, removed Elm hot-reload hacks. MTES-MCT/ecobalyse#62

Merged

n1k0 added a commit to MTES-MCT/ecobalyse that referenced this pull request Jan 18, 2022

Upgrade Parcel to v2.2.1, removed Elm hot-reload hacks. (#62)

3f57d0c

parcel-bundler/parcel#7547 🎉

TLATER mentioned this pull request Jun 23, 2022

Parcel V2 - Sass changes do not update in DIST folder #5965

Open

Don't discard invalidations when transformer throws an error #7547

Don't discard invalidations when transformer throws an error #7547

Conversation

mischnic commented Jan 10, 2022

height bot commented Jan 10, 2022

parcel-benchmark commented Jan 10, 2022 • edited

Benchmark Results

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

mischnic commented Jan 10, 2022 • edited

mischnic commented Jan 12, 2022

objarni commented Jan 16, 2022

parcel-benchmark commented Jan 16, 2022

Benchmark Results

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

Timings

Cold Bundles

Cached Bundles

n1k0 commented Jan 17, 2022 • edited

objarni commented Jan 18, 2022 • edited

w0rm commented Jan 19, 2022

objarni commented Jan 19, 2022

w0rm commented Jan 19, 2022

parcel-benchmark commented Jan 10, 2022 •

edited

mischnic commented Jan 10, 2022 •

edited

n1k0 commented Jan 17, 2022 •

edited

objarni commented Jan 18, 2022 •

edited