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(wire-service): error with invalid adapter id #475

Merged
merged 1 commit into from
Jul 9, 2018

Conversation

kevinv11n
Copy link
Contributor

@kevinv11n kevinv11n commented Jul 6, 2018

  • In non-prod mode, throw an error when the adapter is invalid: non-truthy or unregistered
  • Move existing asserts to be non-prod mode only

Does this PR introduce a breaking change?

  • Yes
  • No

@kevinv11n kevinv11n requested a review from trevor-bliss July 6, 2018 03:29
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 34f0d85 | Target commit: 747e58c

lwc-engine-benchmark

table-append-1k metric base(34f0d85) target(747e58c) trend
benchmark-table/append/1k duration 151.80 (± 6.10 ms) 155.00 (± 5.80 ms) -2.11% 👌
table-clear-1k metric base(34f0d85) target(747e58c) trend
benchmark-table/clear/1k duration 12.30 (± 0.50 ms) 12.60 (± 0.80 ms) -2.44% 👌
table-create-10k metric base(34f0d85) target(747e58c) trend
benchmark-table/create/10k duration 869.30 (± 7.90 ms) 896.60 (± 9.30 ms) -3.14% 👎
table-create-1k metric base(34f0d85) target(747e58c) trend
benchmark-table/create/1k duration 102.90 (± 1.50 ms) 105.20 (± 1.70 ms) -2.24% 👎
table-update-10th-1k metric base(34f0d85) target(747e58c) trend
benchmark-table/update-10th/1k duration 91.50 (± 3.40 ms) 85.30 (± 3.80 ms) 6.78% 👌
tablecmp-append-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-component/append/1k duration 235.10 (± 5.00 ms) 230.50 (± 5.10 ms) 1.96% 👍
tablecmp-clear-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-component/clear/1k duration 37.30 (± 1.65 ms) 36.25 (± 1.75 ms) 2.82% 👌
tablecmp-create-10k metric base(34f0d85) target(747e58c) trend
benchmark-table-component/create/10k duration 1592.30 (± 8.70 ms) 1588.20 (± 16.50 ms) 0.26% 👌
tablecmp-create-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-component/create/1k duration 174.50 (± 4.40 ms) 173.20 (± 3.40 ms) 0.74% 👌
tablecmp-update-10th-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-component/update-10th/1k duration 79.60 (± 4.80 ms) 77.90 (± 3.80 ms) 2.14% 👌
wc-append-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-wc/append/1k duration 259.80 (± 18.50 ms) 270.60 (± 9.50 ms) -4.16% 👌
wc-clear-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-wc/clear/1k duration 39.30 (± 1.20 ms) 38.50 (± 1.30 ms) 2.04% 👍
wc-create-10k metric base(34f0d85) target(747e58c) trend
benchmark-table-wc/create/10k duration 2080.80 (± 7.60 ms) 2061.20 (± 10.50 ms) 0.94% 👍
wc-create-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-wc/create/1k duration 216.00 (± 4.10 ms) 221.80 (± 3.50 ms) -2.69% 👎
wc-update-10th-1k metric base(34f0d85) target(747e58c) trend
benchmark-table-wc/update-10th/1k duration 75.60 (± 6.00 ms) 77.35 (± 5.20 ms) -2.31% 👌

@kevinv11n kevinv11n force-pushed the kv/wire-service-errors-on-nonexistent-adapter branch from 747e58c to cf26759 Compare July 7, 2018 04:09
In non-prod mode, throw an error when the adapter is is invalid: non-truthy or unregistered
@kevinv11n kevinv11n force-pushed the kv/wire-service-errors-on-nonexistent-adapter branch from cf26759 to 9a7d2be Compare July 7, 2018 04:19
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 8169eba | Target commit: cf26759

lwc-engine-benchmark

table-append-1k metric base(8169eba) target(cf26759) trend
benchmark-table/append/1k duration 153.10 (± 5.10 ms) 158.50 (± 5.10 ms) -3.53% 👎
table-clear-1k metric base(8169eba) target(cf26759) trend
benchmark-table/clear/1k duration 11.80 (± 0.70 ms) 12.80 (± 0.80 ms) -8.47% 👎
table-create-10k metric base(8169eba) target(cf26759) trend
benchmark-table/create/10k duration 902.55 (± 10.90 ms) 919.50 (± 7.40 ms) -1.88% 👎
table-create-1k metric base(8169eba) target(cf26759) trend
benchmark-table/create/1k duration 101.80 (± 1.70 ms) 106.50 (± 2.50 ms) -4.62% 👎
table-update-10th-1k metric base(8169eba) target(cf26759) trend
benchmark-table/update-10th/1k duration 85.50 (± 4.50 ms) 88.40 (± 5.60 ms) -3.39% 👎
tablecmp-append-1k metric base(8169eba) target(cf26759) trend
benchmark-table-component/append/1k duration 235.90 (± 4.20 ms) 234.80 (± 3.70 ms) 0.47% 👌
tablecmp-clear-1k metric base(8169eba) target(cf26759) trend
benchmark-table-component/clear/1k duration 35.65 (± 2.05 ms) 38.75 (± 2.25 ms) -8.70% 👎
tablecmp-create-10k metric base(8169eba) target(cf26759) trend
benchmark-table-component/create/10k duration 1597.10 (± 7.90 ms) 1669.70 (± 12.70 ms) -4.55% 👎
tablecmp-create-1k metric base(8169eba) target(cf26759) trend
benchmark-table-component/create/1k duration 169.70 (± 4.35 ms) 175.40 (± 3.50 ms) -3.36% 👎
tablecmp-update-10th-1k metric base(8169eba) target(cf26759) trend
benchmark-table-component/update-10th/1k duration 78.40 (± 4.30 ms) 79.20 (± 3.10 ms) -1.02% 👌
wc-append-1k metric base(8169eba) target(cf26759) trend
benchmark-table-wc/append/1k duration 255.40 (± 14.70 ms) 267.70 (± 14.40 ms) -4.82% 👎
wc-clear-1k metric base(8169eba) target(cf26759) trend
benchmark-table-wc/clear/1k duration 36.40 (± 1.40 ms) 38.20 (± 1.80 ms) -4.95% 👎
wc-create-10k metric base(8169eba) target(cf26759) trend
benchmark-table-wc/create/10k duration 2102.30 (± 11.10 ms) 2153.50 (± 14.80 ms) -2.44% 👎
wc-create-1k metric base(8169eba) target(cf26759) trend
benchmark-table-wc/create/1k duration 215.20 (± 3.50 ms) 217.80 (± 3.40 ms) -1.21% 👌
wc-update-10th-1k metric base(8169eba) target(cf26759) trend
benchmark-table-wc/update-10th/1k duration 75.30 (± 4.60 ms) 78.70 (± 5.00 ms) -4.52% 👎

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 8169eba | Target commit: 9a7d2be

lwc-engine-benchmark

table-append-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table/append/1k duration 153.10 (± 5.10 ms) 157.90 (± 6.00 ms) -3.14% 👎
table-clear-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table/clear/1k duration 11.80 (± 0.70 ms) 11.95 (± 0.65 ms) -1.27% 👌
table-create-10k metric base(8169eba) target(9a7d2be) trend
benchmark-table/create/10k duration 902.55 (± 10.90 ms) 924.70 (± 11.60 ms) -2.45% 👎
table-create-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table/create/1k duration 101.80 (± 1.70 ms) 103.85 (± 3.45 ms) -2.01% 👎
table-update-10th-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table/update-10th/1k duration 85.50 (± 4.50 ms) 90.50 (± 5.60 ms) -5.85% 👎
tablecmp-append-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-component/append/1k duration 235.90 (± 4.20 ms) 242.40 (± 3.50 ms) -2.76% 👎
tablecmp-clear-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-component/clear/1k duration 35.65 (± 2.05 ms) 36.20 (± 1.80 ms) -1.54% 👌
tablecmp-create-10k metric base(8169eba) target(9a7d2be) trend
benchmark-table-component/create/10k duration 1597.10 (± 7.90 ms) 1659.90 (± 12.50 ms) -3.93% 👎
tablecmp-create-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-component/create/1k duration 169.70 (± 4.35 ms) 179.50 (± 4.00 ms) -5.77% 👎
tablecmp-update-10th-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-component/update-10th/1k duration 78.40 (± 4.30 ms) 81.20 (± 4.30 ms) -3.57% 👌
wc-append-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-wc/append/1k duration 255.40 (± 14.70 ms) 259.10 (± 14.90 ms) -1.45% 👌
wc-clear-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-wc/clear/1k duration 36.40 (± 1.40 ms) 38.20 (± 1.50 ms) -4.95% 👎
wc-create-10k metric base(8169eba) target(9a7d2be) trend
benchmark-table-wc/create/10k duration 2102.30 (± 11.10 ms) 2104.30 (± 7.90 ms) -0.10% 👌
wc-create-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-wc/create/1k duration 215.20 (± 3.50 ms) 219.20 (± 5.10 ms) -1.86% 👎
wc-update-10th-1k metric base(8169eba) target(9a7d2be) trend
benchmark-table-wc/update-10th/1k duration 75.30 (± 4.60 ms) 76.30 (± 5.70 ms) -1.33% 👌

if (process.env.NODE_ENV !== 'production') {
assert.isTrue(wireDef.adapter, `@wire on "${wireTarget}": adapter id must be truthy`);
assert.isTrue(adapterFactory, `@wire on "${wireTarget}": unknown adapter id: ${String(wireDef.adapter)}`);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the real change: in non-prod, assert adapter id is truthy and registered (so adapterFactory exists)

if (process.env.NODE_ENV !== 'production') {
assert.isTrue(adapterId, 'adapter id must be truthy');
assert.isTrue(typeof adapterFactory === 'function', 'adapter factory must be a callable');
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move existing asserts under mode check so they're removed in prod

assert.isFalse(connectedListeners.includes(listener as NoArgumentListener), 'must not call addEventListener("connect") with the same listener');
if (process.env.NODE_ENV !== 'production') {
assert.isFalse(connectedListeners.includes(listener as NoArgumentListener), 'must not call addEventListener("connect") with the same listener');
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same with these existing asserts

@kevinv11n kevinv11n merged commit bb0a064 into master Jul 9, 2018
@kevinv11n kevinv11n deleted the kv/wire-service-errors-on-nonexistent-adapter branch July 9, 2018 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants