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

fix(engine): preserve order of event dispaching between CE and SR #309

Merged
merged 4 commits into from
May 24, 2018

Conversation

caridy
Copy link
Contributor

@caridy caridy commented May 17, 2018

Details

This PR solves an existing issue that events added via template will be dispatched between events added via this.template.addEventListener. In ShadowDOM this order is strict, and the shadow root instance (template in this case) should always be dispatched first.

Does this PR introduce a breaking change?

  • No

Pending

  • add more tests

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 7413286 | Target commit: 0d286e7

lwc-engine-benchmark

table-append-1k metric base(7413286) target(0d286e7) trend
benchmark-table/append/1k duration 146.60 (± 3.30 ms) 144.50 (± 3.80 ms) 1.43% 👍
table-clear-1k metric base(7413286) target(0d286e7) trend
benchmark-table/clear/1k duration 11.40 (± 0.60 ms) 11.30 (± 0.40 ms) 0.88% 👌
table-create-10k metric base(7413286) target(0d286e7) trend
benchmark-table/create/10k duration 857.00 (± 5.50 ms) 838.60 (± 7.40 ms) 2.15% 👍
table-create-1k metric base(7413286) target(0d286e7) trend
benchmark-table/create/1k duration 101.00 (± 1.50 ms) 101.40 (± 2.00 ms) -0.40% 👌
table-update-10th-1k metric base(7413286) target(0d286e7) trend
benchmark-table/update-10th/1k duration 90.00 (± 5.50 ms) 87.60 (± 4.40 ms) 2.67% 👌
tablecmp-append-1k metric base(7413286) target(0d286e7) trend
benchmark-table-component/append/1k duration 246.00 (± 4.10 ms) 254.30 (± 4.50 ms) -3.37% 👎
tablecmp-clear-1k metric base(7413286) target(0d286e7) trend
benchmark-table/clear/1k duration 31.20 (± 1.60 ms) 35.20 (± 1.50 ms) -12.82% 👎
tablecmp-create-10k metric base(7413286) target(0d286e7) trend
benchmark-table-component/create/10k duration 1696.60 (± 10.40 ms) 1695.30 (± 11.00 ms) 0.08% 👌
tablecmp-create-1k metric base(7413286) target(0d286e7) trend
benchmark-table-component/create/1k duration 190.30 (± 3.80 ms) 196.20 (± 4.40 ms) -3.10% 👎
tablecmp-update-10th-1k metric base(7413286) target(0d286e7) trend
benchmark-table-component/update-10th/1k duration 75.30 (± 4.00 ms) 75.40 (± 4.30 ms) -0.13% 👌

// it is dispatched onto the custom element directly
target === currentTarget ||
// it is coming from an slotted element
isChildNode(getRootNode.call(target, event), currentTarget as Node)
Copy link
Contributor

Choose a reason for hiding this comment

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

Pass { composed: false } object instead of event

caridy added a commit that referenced this pull request May 17, 2018
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 6ad80ca | Target commit: 21aa73a

lwc-engine-benchmark

table-append-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table/append/1k duration 151.80 (± 6.00 ms) 146.90 (± 4.40 ms) 3.23% 👍
table-clear-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table/clear/1k duration 11.90 (± 0.50 ms) 11.20 (± 0.30 ms) 5.88% 👍
table-create-10k metric base(6ad80ca) target(21aa73a) trend
benchmark-table/create/10k duration 843.60 (± 6.50 ms) 845.20 (± 5.70 ms) -0.19% 👌
table-create-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table/create/1k duration 102.60 (± 1.50 ms) 102.10 (± 1.90 ms) 0.49% 👌
table-update-10th-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table/update-10th/1k duration 90.35 (± 5.55 ms) 88.00 (± 5.30 ms) 2.60% 👌
tablecmp-append-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table-component/append/1k duration 213.10 (± 14.50 ms) 257.10 (± 3.80 ms) -20.65% 👎
tablecmp-clear-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table/clear/1k duration 31.90 (± 1.40 ms) 34.50 (± 1.20 ms) -8.15% 👎
tablecmp-create-10k metric base(6ad80ca) target(21aa73a) trend
benchmark-table-component/create/10k duration 1701.20 (± 11.20 ms) 1697.50 (± 9.00 ms) 0.22% 👌
tablecmp-create-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table-component/create/1k duration 192.90 (± 4.10 ms) 193.80 (± 3.10 ms) -0.47% 👌
tablecmp-update-10th-1k metric base(6ad80ca) target(21aa73a) trend
benchmark-table-component/update-10th/1k duration 77.50 (± 4.70 ms) 74.40 (± 4.10 ms) 4.00% 👌

@caridy
Copy link
Contributor Author

caridy commented May 22, 2018

@davidturissini @ekashida can you guys take care of this one? I still some perf regression.

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 51adcb6 | Target commit: c72daf5

lwc-engine-benchmark

table-append-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table/append/1k duration 145.00 (± 5.60 ms) 146.50 (± 3.50 ms) -1.03% 👌
table-clear-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table/clear/1k duration 11.80 (± 0.50 ms) 11.60 (± 0.50 ms) 1.69% 👌
table-create-10k metric base(51adcb6) target(c72daf5) trend
benchmark-table/create/10k duration 844.30 (± 3.70 ms) 834.70 (± 4.90 ms) 1.14% 👍
table-create-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table/create/1k duration 101.70 (± 2.00 ms) 100.10 (± 1.90 ms) 1.57% 👍
table-update-10th-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table/update-10th/1k duration 92.50 (± 5.15 ms) 91.40 (± 5.10 ms) 1.19% 👌
tablecmp-append-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table-component/append/1k duration 242.10 (± 5.20 ms) 258.00 (± 4.70 ms) -6.57% 👎
tablecmp-clear-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table/clear/1k duration 33.00 (± 1.10 ms) 35.40 (± 1.60 ms) -7.27% 👎
tablecmp-create-10k metric base(51adcb6) target(c72daf5) trend
benchmark-table-component/create/10k duration 1712.50 (± 6.50 ms) 1686.60 (± 13.10 ms) 1.51% 👍
tablecmp-create-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table-component/create/1k duration 188.70 (± 3.90 ms) 197.50 (± 4.20 ms) -4.66% 👎
tablecmp-update-10th-1k metric base(51adcb6) target(c72daf5) trend
benchmark-table-component/update-10th/1k duration 74.40 (± 5.10 ms) 75.00 (± 4.20 ms) -0.81% 👌

davidturissini pushed a commit that referenced this pull request May 24, 2018
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 1e4c7f1 | Target commit: 59b17f4

lwc-engine-benchmark

table-append-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table/append/1k duration 147.90 (± 4.80 ms) 146.70 (± 3.40 ms) 0.81% 👌
table-clear-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table/clear/1k duration 12.00 (± 0.40 ms) 11.60 (± 0.40 ms) 3.33% 👍
table-create-10k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table/create/10k duration 865.40 (± 6.00 ms) 850.10 (± 4.00 ms) 1.77% 👍
table-create-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table/create/1k duration 101.40 (± 1.30 ms) 100.50 (± 1.70 ms) 0.89% 👌
table-update-10th-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table/update-10th/1k duration 89.30 (± 4.20 ms) 92.20 (± 5.10 ms) -3.25% 👌
tablecmp-append-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table-component/append/1k duration 250.00 (± 4.10 ms) 260.20 (± 5.10 ms) -4.08% 👎
tablecmp-clear-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table/clear/1k duration 33.45 (± 1.45 ms) 35.60 (± 1.70 ms) -6.43% 👎
tablecmp-create-10k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table-component/create/10k duration 1709.40 (± 7.80 ms) 1728.10 (± 14.90 ms) -1.09% 👎
tablecmp-create-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table-component/create/1k duration 193.90 (± 5.10 ms) 199.60 (± 3.10 ms) -2.94% 👎
tablecmp-update-10th-1k metric base(1e4c7f1) target(59b17f4) trend
benchmark-table-component/update-10th/1k duration 74.00 (± 3.30 ms) 76.30 (± 5.40 ms) -3.11% 👌

@davidturissini
Copy link
Contributor

Tests added in #339

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 3e3a15d | Target commit: 464d5c0

lwc-engine-benchmark

table-append-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table/append/1k duration 145.80 (± 4.00 ms) 144.40 (± 4.50 ms) 0.96% 👍
table-clear-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table/clear/1k duration 11.50 (± 0.40 ms) 11.20 (± 0.50 ms) 2.61% 👍
table-create-10k metric base(3e3a15d) target(464d5c0) trend
benchmark-table/create/10k duration 847.00 (± 3.90 ms) 845.50 (± 4.70 ms) 0.18% 👌
table-create-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table/create/1k duration 102.00 (± 2.10 ms) 102.20 (± 1.60 ms) -0.20% 👌
table-update-10th-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table/update-10th/1k duration 86.00 (± 5.20 ms) 91.30 (± 5.30 ms) -6.16% 👌
tablecmp-append-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table-component/append/1k duration 245.90 (± 2.40 ms) 258.20 (± 4.60 ms) -5.00% 👎
tablecmp-clear-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table/clear/1k duration 32.00 (± 1.30 ms) 35.00 (± 1.90 ms) -9.38% 👎
tablecmp-create-10k metric base(3e3a15d) target(464d5c0) trend
benchmark-table-component/create/10k duration 1702.60 (± 11.65 ms) 1710.00 (± 13.50 ms) -0.43% 👌
tablecmp-create-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table-component/create/1k duration 190.40 (± 3.70 ms) 197.40 (± 3.80 ms) -3.68% 👎
tablecmp-update-10th-1k metric base(3e3a15d) target(464d5c0) trend
benchmark-table-component/update-10th/1k duration 72.90 (± 4.30 ms) 74.55 (± 4.05 ms) -2.26% 👌

@davidturissini davidturissini merged commit 1a1ba17 into master May 24, 2018
@davidturissini davidturissini deleted the caridy/fix-listener-order branch May 24, 2018 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants