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

Always add newline after code in scopehoisting packager #3770

Merged
merged 7 commits into from
Nov 15, 2019
Merged

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Nov 12, 2019

↪️ Pull Request

The kitchensink example (!) was failing because the scopehoisting packager had this output:

(function(){let a="hi!";console.log(a);// const message = require('./message');
// const fs = require('fs');
// console.log(message); // eslint-disable-line no-console
// console.log(fs.readFileSync(__dirname + '/test.txt', 'utf8'));
// class Test {}  })();

We need to always add a newline after the code.

@parcel-benchmark
Copy link

parcel-benchmark commented Nov 12, 2019

Benchmark Results

packages/benchmarks/kitchen-sink ✅

Timings

Description Time Difference
Cold 11.54s +11.54s ⚠️
Cached 9.85s +9.85s ⚠️

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

packages/benchmarks/react-hn ✅

Timings

Description Time Difference
Cold 16.91s -308.00ms
Cached 14.44s -149.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/main/index.js 34.53kb +0.00b 14.00ms -1.00ms 🚀
dist/module/index.js 34.33kb +0.00b 9.00ms -1.00ms 🚀

packages/benchmarks/ak-editor ✅

Timings

Description Time Difference
Cold 4.10m -141.00ms
Cached 12.48s -46.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/index.js 2.31mb -312.00b 🚀 2.36m +3.68s
dist/media-viewer.12bd9010.js 86.76kb +0.00b 8.00s -827.00ms 🚀
dist/card.80c210d3.js 76.35kb +0.00b 8.00s -827.00ms 🚀
dist/popup.e57e7ab5.js 43.88kb +0.00b 51.13s +3.69s ⚠️
dist/ResourcedEmojiComponent.2d80adbd.js 2.25kb +0.00b 7.92s -851.00ms 🚀
dist/esm.976debcc.js 178.00b +0.00b 7.87s -830.00ms 🚀
dist/dropzone.976debcc.js 177.00b +1.00b ⚠️ 2.31m +1.46m ⚠️
dist/clipboard.976debcc.js 177.00b +1.00b ⚠️ 53.31s +2.01s
dist/browser.976debcc.js 177.00b +1.00b ⚠️ 1.27m +25.11s ⚠️
dist/editorView.976debcc.js 177.00b -1.00b 🚀 52.03s -24.11s 🚀
dist/media-card-analytics-error-boundary.976debcc.js 176.00b -5.00b 🚀 54.04s -1.39m 🚀
dist/16.976debcc.js 174.00b -5.00b 🚀 52.70s -98.00ms

Cached Bundles

Bundle Size Difference Time Difference
dist/index.js 2.32mb -312.00b 🚀 409.00ms -9.00ms
dist/smartMediaEditor.29d5f169.js 601.41kb +0.00b 176.00ms -14.00ms 🚀
dist/EmojiPickerComponent.7202df5c.js 105.88kb +0.00b 140.00ms -8.00ms 🚀
dist/media-viewer.12bd9010.js 86.76kb +0.00b 189.00ms +16.00ms ⚠️
dist/card.80c210d3.js 76.35kb +0.00b 189.00ms +17.00ms ⚠️
dist/popup.37645d0d.js 44.40kb +0.00b 168.00ms +12.00ms ⚠️
dist/popup.e57e7ab5.js 43.88kb +0.00b 168.00ms +14.00ms ⚠️
dist/popup.de16c573.js 40.69kb +0.00b 167.00ms +14.00ms ⚠️
dist/popup.742fc489.js 37.84kb +0.00b 171.00ms +13.00ms ⚠️
dist/esm.d429ab19.js 25.72kb +0.00b 166.00ms +13.00ms ⚠️
dist/js.adf5baf7.js 16.54kb +0.00b 172.00ms +13.00ms ⚠️
dist/workerHasher.8269e4ca.js 11.56kb +0.00b 169.00ms +12.00ms ⚠️
dist/card.f99e1597.js 5.89kb +0.00b 173.00ms +13.00ms ⚠️
dist/png-chunks-extract.3d8d8594.js 3.33kb +0.00b 172.00ms +13.00ms ⚠️
dist/ResourcedEmojiComponent.2d80adbd.js 2.25kb +0.00b 176.00ms +13.00ms ⚠️
dist/media-picker-analytics-error-boundary.c660ebe7.js 1.92kb +0.00b 166.00ms +14.00ms ⚠️
dist/card.9520f336.js 1.80kb +0.00b 173.00ms +13.00ms ⚠️
dist/16.dcacbe5c.js 1.69kb +0.00b 154.00ms +9.00ms ⚠️
dist/date.cfe6153f.js 1.63kb +0.00b 151.00ms +9.00ms ⚠️
dist/16.de3f3184.js 1.62kb +0.00b 157.00ms +9.00ms ⚠️
dist/workerHasher.7f0e2d92.js 1.48kb +0.00b 170.00ms +12.00ms ⚠️
dist/16.f54dac28.js 1.34kb +0.00b 163.00ms +17.00ms ⚠️
dist/16.8f0e8fe1.js 1.29kb +0.00b 157.00ms +8.00ms ⚠️
dist/16.43b62c01.js 1.29kb +0.00b 156.00ms +9.00ms ⚠️
dist/code.092e0137.js 1.28kb +0.00b 151.00ms +8.00ms ⚠️
dist/16.b17a4d30.js 1.27kb +0.00b 153.00ms +9.00ms ⚠️
dist/16.51bbc68e.js 1.23kb +0.00b 152.00ms +8.00ms ⚠️
dist/16.385d3ff5.js 1.19kb +0.00b 153.00ms +9.00ms ⚠️
dist/16.b3ce4050.js 1.16kb +0.00b 156.00ms +8.00ms ⚠️
dist/16.e37bbe6b.js 1.15kb +0.00b 153.00ms +8.00ms ⚠️
dist/16.f7fe274a.js 1.14kb +0.00b 152.00ms +9.00ms ⚠️
dist/16.ac2ff83f.js 1.10kb +0.00b 154.00ms +9.00ms ⚠️
dist/16.24165259.js 1.09kb +0.00b 156.00ms +8.00ms ⚠️
dist/16.97c5bdf9.js 1.09kb +0.00b 154.00ms +9.00ms ⚠️
dist/divider.160f3108.js 940.00b +0.00b 150.00ms +9.00ms ⚠️
dist/action.146450ba.js 919.00b +0.00b 151.00ms +8.00ms ⚠️
dist/decision.06254b93.js 898.00b +0.00b 150.00ms +8.00ms ⚠️
dist/media-viewer-analytics-error-boundary.dd752a9e.js 777.00b +0.00b 174.00ms +13.00ms ⚠️
dist/simpleHasher.efc670cc.js 446.00b +0.00b 168.00ms +12.00ms ⚠️
dist/esm.976debcc.js 176.00b +2.00b ⚠️ 176.00ms +13.00ms ⚠️
dist/media-card-analytics-error-boundary.976debcc.js 176.00b +2.00b ⚠️ 172.00ms +12.00ms ⚠️
dist/dropzone.976debcc.js 176.00b +2.00b ⚠️ 166.00ms +13.00ms ⚠️
dist/clipboard.976debcc.js 176.00b +2.00b ⚠️ 165.00ms +13.00ms ⚠️
dist/browser.976debcc.js 176.00b +2.00b ⚠️ 154.00ms +2.00ms
dist/16.976debcc.js 176.00b +2.00b ⚠️ 152.00ms +9.00ms ⚠️
dist/editorView.976debcc.js 176.00b +2.00b ⚠️ 129.00ms -1.00ms
dist/card.aae6ebb9.js 62.00b +0.00b 174.00ms +13.00ms ⚠️
dist/component.ab65829f.js 62.00b +0.00b 160.00ms +10.00ms ⚠️

Click here to view a detailed benchmark overview.

@mischnic
Copy link
Member Author

mischnic commented Nov 12, 2019

(This also fixes the first benchmark, I guess I should add an integration test as well.)

@DeMoorJasper
Copy link
Member

An extra test would be great to prevent these regressions in the future :)

@mischnic mischnic merged commit 4a449dc into v2 Nov 15, 2019
@mischnic mischnic deleted the iife-nl branch November 15, 2019 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants