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: Ensure uniqueness of public properties are compile time #323

Merged
merged 2 commits into from
May 21, 2018

Conversation

pmdartus
Copy link
Member

Details

This PR makes compilation fail when a component defines multiple @api public properties with the same name. Currently, the compiler discards the redundant property definition and only keep the last one.

Example of failing code:

import { api } from 'engine';
export default class Text {
	@api foo = 1;
	@api foo = 2;
 // ^ Duplicate @api property "foo".
}

Does this PR introduce a breaking change?

  • Yes
  • No

@pmdartus pmdartus requested a review from diervo May 21, 2018 16:56
@pmdartus pmdartus changed the title Pmdartus/duplicate api definition fix: Ensure uniqueness of public properties are compile time. May 21, 2018
@pmdartus pmdartus changed the title fix: Ensure uniqueness of public properties are compile time. fix: Ensure uniqueness of public properties are compile time May 21, 2018
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 948fbb9 | Target commit: d231f80

lwc-engine-benchmark

table-append-1k metric base(948fbb9) target(d231f80) trend
benchmark-table/append/1k duration 142.80 (± 3.90 ms) 144.00 (± 4.10 ms) -0.84% 👌
table-clear-1k metric base(948fbb9) target(d231f80) trend
benchmark-table/clear/1k duration 11.80 (± 0.60 ms) 12.10 (± 0.50 ms) -2.54% 👌
table-create-10k metric base(948fbb9) target(d231f80) trend
benchmark-table/create/10k duration 838.70 (± 8.00 ms) 842.20 (± 6.90 ms) -0.42% 👎
table-create-1k metric base(948fbb9) target(d231f80) trend
benchmark-table/create/1k duration 101.00 (± 2.30 ms) 102.95 (± 1.85 ms) -1.93% 👎
table-update-10th-1k metric base(948fbb9) target(d231f80) trend
benchmark-table/update-10th/1k duration 87.70 (± 4.50 ms) 91.70 (± 5.30 ms) -4.56% 👌
tablecmp-append-1k metric base(948fbb9) target(d231f80) trend
benchmark-table-component/append/1k duration 242.10 (± 4.90 ms) 242.10 (± 5.80 ms) 0.00% 👌
tablecmp-clear-1k metric base(948fbb9) target(d231f80) trend
benchmark-table/clear/1k duration 30.80 (± 1.10 ms) 30.70 (± 1.20 ms) 0.32% 👌
tablecmp-create-10k metric base(948fbb9) target(d231f80) trend
benchmark-table-component/create/10k duration 1680.80 (± 12.20 ms) 1691.30 (± 8.60 ms) -0.62% 👎
tablecmp-create-1k metric base(948fbb9) target(d231f80) trend
benchmark-table-component/create/1k duration 186.70 (± 4.30 ms) 188.40 (± 3.40 ms) -0.91% 👌
tablecmp-update-10th-1k metric base(948fbb9) target(d231f80) trend
benchmark-table-component/update-10th/1k duration 73.30 (± 4.50 ms) 74.20 (± 3.80 ms) -1.23% 👌

@diervo
Copy link
Contributor

diervo commented May 21, 2018

@pmdartus O(n^2) #pieroperf :)

@pmdartus pmdartus merged commit bf88354 into master May 21, 2018
@pmdartus pmdartus deleted the pmdartus/duplicate-api-definition branch May 21, 2018 17:39
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.

2 participants