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

feat: remove backwards compatibility support for "engine" #721

Merged
merged 3 commits into from
Oct 11, 2018
Merged

feat: remove backwards compatibility support for "engine" #721

merged 3 commits into from
Oct 11, 2018

Conversation

apapko
Copy link
Collaborator

@apapko apapko commented Oct 9, 2018 •
edited
Loading

Details

Remove backwards compatibility support for using:

import { Element } from 'engine';

Fixes #719

The 'engine' will be treated as external module and the class won't be transformed into a LWC component ( no html injection nor render method ).

Does this PR introduce a breaking change?

  • Yes
  • No

Components that used to extend their class from Element will no longer work.

@apapko apapko requested review from diervo and pmdartus October 9, 2018 05:24
kevinv11n
kevinv11n approved these changes Oct 9, 2018
View reviewed changes
Copy link
Contributor

@kevinv11n kevinv11n left a comment

Choose a reason for hiding this comment

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

Nit on the PR subject: it's not a revert in the git sense, it's a remove

diervo
diervo suggested changes Oct 9, 2018
View reviewed changes
Copy link
Contributor

@diervo diervo left a comment

Choose a reason for hiding this comment

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

I believe there are more occurrences in other packages like the jest resolver, and also some configurations in the package.json as well.

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 7a22a8b | Target commit: 2f57d6f

lwc-engine-benchmark

table-append-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table/append/1k duration 158.35 (±7.10 ms) 163.30 (±4.50 ms) +4.9ms (3.1%) 👎
table-clear-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table/clear/1k duration 12.65 (±0.65 ms) 12.90 (±0.80 ms) +0.3ms (2.0%) 👌
table-create-10k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table/create/10k duration 881.35 (±6.75 ms) 959.20 (±7.00 ms) +77.9ms (8.8%) 👎
table-create-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table/create/1k duration 110.05 (±2.20 ms) 113.40 (±2.15 ms) +3.4ms (3.0%) 👎
table-update-10th-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table/update-10th/1k duration 86.65 (±2.40 ms) 99.45 (±1.95 ms) +12.8ms (14.8%) 👎
tablecmp-append-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-component/append/1k duration 203.85 (±7.15 ms) 238.20 (±8.75 ms) +34.3ms (16.9%) 👎
tablecmp-clear-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-component/clear/1k duration 20.30 (±1.45 ms) 22.40 (±2.00 ms) +2.1ms (10.3%) 👎
tablecmp-create-10k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-component/create/10k duration 1613.75 (±9.95 ms) 1671.90 (±8.10 ms) +58.2ms (3.6%) 👎
tablecmp-create-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-component/create/1k duration 186.65 (±4.75 ms) 193.25 (±5.20 ms) +6.6ms (3.5%) 👎
tablecmp-update-10th-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-component/update-10th/1k duration 82.40 (±5.00 ms) 84.20 (±6.30 ms) +1.8ms (2.2%) 👌
wc-append-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-wc/append/1k duration 264.65 (±18.90 ms) 270.80 (±20.90 ms) +6.2ms (2.3%) 👌
wc-clear-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-wc/clear/1k duration 29.50 (±2.75 ms) 31.05 (±2.65 ms) +1.6ms (5.3%) 👎
wc-create-10k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-wc/create/10k duration 2064.60 (±12.20 ms) 2196.95 (±13.40 ms) +132.3ms (6.4%) 👎
wc-create-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-wc/create/1k duration 222.00 (±6.00 ms) 227.60 (±5.15 ms) +5.6ms (2.5%) 👎
wc-update-10th-1k metric base(7a22a8b) target(2f57d6f) trend
benchmark-table-wc/update-10th/1k duration 85.40 (±8.00 ms) 87.35 (±6.05 ms) +1.9ms (2.3%) 👌

@apapko
Copy link
Collaborator Author

apapko commented Oct 9, 2018

@diervo thanks for the heads up! I will take a look

@apapko apapko changed the title feat: revert backwards compatibility support for "engine" feat: remove backwards compatibility support for "engine" Oct 9, 2018
@apapko apapko requested a review from trevor-bliss October 9, 2018 17:12
@diervo
Copy link
Contributor

diervo commented Oct 9, 2018

Not yet! There are more missing :)

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 7a22a8b | Target commit: 377d20c

lwc-engine-benchmark

table-append-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/append/1k duration 158.35 (±7.10 ms) 174.80 (±4.45 ms) +16.4ms (10.4%) 👎
table-clear-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/clear/1k duration 12.65 (±0.65 ms) 13.65 (±0.95 ms) +1.0ms (7.9%) 👎
table-create-10k metric base(7a22a8b) target(377d20c) trend
benchmark-table/create/10k duration 881.35 (±6.75 ms) 963.50 (±8.25 ms) +82.2ms (9.3%) 👎
table-create-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/create/1k duration 110.05 (±2.20 ms) 115.10 (±2.25 ms) +5.0ms (4.6%) 👎
table-update-10th-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/update-10th/1k duration 86.65 (±2.40 ms) 91.45 (±3.45 ms) +4.8ms (5.5%) 👎
tablecmp-append-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/append/1k duration 203.85 (±7.15 ms) 258.15 (±7.20 ms) +54.3ms (26.6%) 👎
tablecmp-clear-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/clear/1k duration 20.30 (±1.45 ms) 24.25 (±1.50 ms) +3.9ms (19.5%) 👎
tablecmp-create-10k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/create/10k duration 1613.75 (±9.95 ms) 1749.60 (±15.50 ms) +135.8ms (8.4%) 👎
tablecmp-create-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/create/1k duration 186.65 (±4.75 ms) 196.20 (±4.90 ms) +9.5ms (5.1%) 👎
tablecmp-update-10th-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/update-10th/1k duration 82.40 (±5.00 ms) 86.60 (±6.45 ms) +4.2ms (5.1%) 👌
wc-append-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/append/1k duration 264.65 (±18.90 ms) 283.60 (±16.95 ms) +19.0ms (7.2%) 👎
wc-clear-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/clear/1k duration 29.50 (±2.75 ms) 32.55 (±2.50 ms) +3.0ms (10.3%) 👎
wc-create-10k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/create/10k duration 2064.60 (±12.20 ms) 2267.35 (±14.60 ms) +202.8ms (9.8%) 👎
wc-create-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/create/1k duration 222.00 (±6.00 ms) 232.40 (±5.85 ms) +10.4ms (4.7%) 👎
wc-update-10th-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/update-10th/1k duration 85.40 (±8.00 ms) 90.60 (±5.75 ms) +5.2ms (6.1%) 👎

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 7a22a8b | Target commit: 377d20c

lwc-engine-benchmark

table-append-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/append/1k duration 158.35 (±7.10 ms) 168.15 (±4.25 ms) +9.8ms (6.2%) 👎
table-clear-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/clear/1k duration 12.65 (±0.65 ms) 13.55 (±0.60 ms) +0.9ms (7.1%) 👎
table-create-10k metric base(7a22a8b) target(377d20c) trend
benchmark-table/create/10k duration 881.35 (±6.75 ms) 967.50 (±8.90 ms) +86.2ms (9.8%) 👎
table-create-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/create/1k duration 110.05 (±2.20 ms) 116.40 (±3.80 ms) +6.4ms (5.8%) 👎
table-update-10th-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table/update-10th/1k duration 86.65 (±2.40 ms) 92.70 (±4.30 ms) +6.0ms (7.0%) 👎
tablecmp-append-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/append/1k duration 203.85 (±7.15 ms) 223.40 (±11.15 ms) +19.5ms (9.6%) 👎
tablecmp-clear-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/clear/1k duration 20.30 (±1.45 ms) 22.25 (±2.20 ms) +1.9ms (9.6%) 👎
tablecmp-create-10k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/create/10k duration 1613.75 (±9.95 ms) 1741.45 (±11.35 ms) +127.7ms (7.9%) 👎
tablecmp-create-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/create/1k duration 186.65 (±4.75 ms) 195.25 (±4.90 ms) +8.6ms (4.6%) 👎
tablecmp-update-10th-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-component/update-10th/1k duration 82.40 (±5.00 ms) 87.25 (±6.50 ms) +4.8ms (5.9%) 👎
wc-append-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/append/1k duration 264.65 (±18.90 ms) 276.55 (±19.45 ms) +11.9ms (4.5%) 👎
wc-clear-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/clear/1k duration 29.50 (±2.75 ms) 31.40 (±2.35 ms) +1.9ms (6.4%) 👎
wc-create-10k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/create/10k duration 2064.60 (±12.20 ms) 2218.05 (±16.60 ms) +153.4ms (7.4%) 👎
wc-create-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/create/1k duration 222.00 (±6.00 ms) 229.40 (±6.00 ms) +7.4ms (3.3%) 👎
wc-update-10th-1k metric base(7a22a8b) target(377d20c) trend
benchmark-table-wc/update-10th/1k duration 85.40 (±8.00 ms) 89.20 (±5.30 ms) +3.8ms (4.4%) 👎

@apapko
Copy link
Collaborator Author

apapko commented Oct 10, 2018

These should all be done (see #719 for all affected repos), but raptorci issue is preventing jest utils and mocks from passing the ci. Posted on the sfci group for help.

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 81e8c2b | Target commit: d56c97e

lwc-engine-benchmark

table-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/append/1k duration 155.35 (±5.70 ms) 157.60 (±5.50 ms) +2.3ms (1.4%) 👎
table-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/clear/1k duration 12.10 (±0.70 ms) 12.15 (±0.60 ms) +0.0ms (0.4%) 👌
table-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/create/10k duration 933.70 (±7.60 ms) 935.10 (±5.90 ms) +1.4ms (0.1%) 👌
table-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/create/1k duration 109.30 (±1.95 ms) 112.85 (±2.25 ms) +3.5ms (3.2%) 👎
table-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/update-10th/1k duration 89.80 (±6.75 ms) 87.75 (±3.90 ms) -2.0ms (2.3%) 👌
tablecmp-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/append/1k duration 224.75 (±10.05 ms) 241.40 (±9.45 ms) +16.7ms (7.4%) 👎
tablecmp-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/clear/1k duration 19.70 (±2.40 ms) 20.50 (±1.75 ms) +0.8ms (4.1%) 👌
tablecmp-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/create/10k duration 1572.95 (±7.50 ms) 1684.85 (±8.95 ms) +111.9ms (7.1%) 👎
tablecmp-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/create/1k duration 184.25 (±6.25 ms) 188.30 (±6.35 ms) +4.1ms (2.2%) 👎
tablecmp-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/update-10th/1k duration 81.90 (±4.35 ms) 82.85 (±6.50 ms) +0.9ms (1.2%) 👌
wc-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/append/1k duration 266.25 (±14.35 ms) 278.90 (±11.25 ms) +12.6ms (4.8%) 👎
wc-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/clear/1k duration 28.25 (±2.80 ms) 30.45 (±3.35 ms) +2.2ms (7.8%) 👎
wc-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/create/10k duration 2048.50 (±12.00 ms) 2187.65 (±14.65 ms) +139.1ms (6.8%) 👎
wc-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/create/1k duration 220.25 (±4.25 ms) 224.35 (±5.15 ms) +4.1ms (1.9%) 👎
wc-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/update-10th/1k duration 80.35 (±6.20 ms) 85.85 (±6.70 ms) +5.5ms (6.8%) 👎

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 81e8c2b | Target commit: d56c97e

lwc-engine-benchmark

table-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/append/1k duration 155.35 (±5.70 ms) 156.85 (±5.35 ms) +1.5ms (1.0%) 👎
table-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/clear/1k duration 12.10 (±0.70 ms) 12.60 (±0.75 ms) +0.5ms (4.1%) 👌
table-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/create/10k duration 933.70 (±7.60 ms) 940.65 (±7.50 ms) +6.9ms (0.7%) 👎
table-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/create/1k duration 109.30 (±1.95 ms) 114.05 (±3.45 ms) +4.8ms (4.3%) 👎
table-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/update-10th/1k duration 89.80 (±6.75 ms) 98.70 (±3.75 ms) +8.9ms (9.9%) 👎
tablecmp-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/append/1k duration 224.75 (±10.05 ms) 241.10 (±5.70 ms) +16.4ms (7.3%) 👎
tablecmp-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/clear/1k duration 19.70 (±2.40 ms) 20.25 (±1.70 ms) +0.6ms (2.8%) 👌
tablecmp-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/create/10k duration 1572.95 (±7.50 ms) 1695.60 (±13.60 ms) +122.6ms (7.8%) 👎
tablecmp-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/create/1k duration 184.25 (±6.25 ms) 188.05 (±5.60 ms) +3.8ms (2.1%) 👎
tablecmp-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/update-10th/1k duration 81.90 (±4.35 ms) 83.95 (±4.40 ms) +2.0ms (2.5%) 👌
wc-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/append/1k duration 266.25 (±14.35 ms) 279.35 (±12.10 ms) +13.1ms (4.9%) 👎
wc-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/clear/1k duration 28.25 (±2.80 ms) 31.45 (±2.05 ms) +3.2ms (11.3%) 👎
wc-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/create/10k duration 2048.50 (±12.00 ms) 2185.50 (±17.85 ms) +137.0ms (6.7%) 👎
wc-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/create/1k duration 220.25 (±4.25 ms) 228.70 (±5.65 ms) +8.4ms (3.8%) 👎
wc-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/update-10th/1k duration 80.35 (±6.20 ms) 86.45 (±6.70 ms) +6.1ms (7.6%) 👎

@apapko apapko merged commit e312f54 into master Oct 11, 2018
@apapko apapko deleted the apapko/remove-engine-support branch October 11, 2018 16:16
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 81e8c2b | Target commit: d56c97e

lwc-engine-benchmark

table-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/append/1k duration 155.35 (±5.70 ms) 160.90 (±4.85 ms) +5.6ms (3.6%) 👎
table-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/clear/1k duration 12.10 (±0.70 ms) 12.75 (±0.85 ms) +0.7ms (5.4%) 👌
table-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/create/10k duration 933.70 (±7.60 ms) 936.80 (±11.55 ms) +3.1ms (0.3%) 👎
table-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/create/1k duration 109.30 (±1.95 ms) 112.95 (±3.55 ms) +3.6ms (3.3%) 👎
table-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table/update-10th/1k duration 89.80 (±6.75 ms) 90.45 (±5.15 ms) +0.6ms (0.7%) 👎
tablecmp-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/append/1k duration 224.75 (±10.05 ms) 241.60 (±6.50 ms) +16.9ms (7.5%) 👎
tablecmp-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/clear/1k duration 19.70 (±2.40 ms) 21.25 (±2.60 ms) +1.6ms (7.9%) 👎
tablecmp-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/create/10k duration 1572.95 (±7.50 ms) 1707.05 (±12.20 ms) +134.1ms (8.5%) 👎
tablecmp-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/create/1k duration 184.25 (±6.25 ms) 189.75 (±5.50 ms) +5.5ms (3.0%) 👎
tablecmp-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-component/update-10th/1k duration 81.90 (±4.35 ms) 81.40 (±5.50 ms) -0.5ms (0.6%) 👌
wc-append-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/append/1k duration 266.25 (±14.35 ms) 267.05 (±16.85 ms) +0.8ms (0.3%) 👌
wc-clear-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/clear/1k duration 28.25 (±2.80 ms) 31.10 (±1.90 ms) +2.9ms (10.1%) 👎
wc-create-10k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/create/10k duration 2048.50 (±12.00 ms) 2190.95 (±15.10 ms) +142.4ms (7.0%) 👎
wc-create-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/create/1k duration 220.25 (±4.25 ms) 232.15 (±6.30 ms) +11.9ms (5.4%) 👎
wc-update-10th-1k metric base(81e8c2b) target(d56c97e) trend
benchmark-table-wc/update-10th/1k duration 80.35 (±6.20 ms) 85.65 (±7.20 ms) +5.3ms (6.6%) 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmdartus pmdartus pmdartus approved these changes

@kevinv11n kevinv11n kevinv11n approved these changes

@diervo diervo diervo approved these changes

@trevor-bliss trevor-bliss Awaiting requested review from trevor-bliss

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

Successfully merging this pull request may close these issues.
4 participants
@apapko @diervo @kevinv11n @pmdartus