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(compiler): don't use project-wide config #1163

Merged
merged 2 commits into from
Apr 27, 2019
Merged

Conversation

midzelis
Copy link
Contributor

@midzelis midzelis commented Apr 8, 2019

Details

Modify the lwc compiler to avoid using the project wide babel.config.js file. This was discovered when adding support to LWC LSP when opening standalone projects. If that standalone project was using a babel.config.js file, it was interfering with the lwc compiler babel transforms, which should be isolated. lwc compiler already pases babelrc: false as a default option. babelConfig: false is the way to disable the project wide config. Project wide configs are new in babel 7.x.

Does this PR introduce a breaking change?

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:
Please check if your PR fulfills the following requirements:

@midzelis midzelis requested review from pmdartus and apapko April 8, 2019 13:37
@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: 74a42fc | Target commit: 1c2878b

lwc-engine-benchmark

table-append-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table/append/1k duration 155.05 (±5.75 ms) 155.15 (±3.95 ms) +0.1ms (0.1%) 👌
table-clear-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table/clear/1k duration 10.80 (±0.65 ms) 10.80 (±0.70 ms) 0.0ms (0.0%) 👌
table-create-10k metric base(74a42fc) target(1c2878b) trend
benchmark-table/create/10k duration 893.45 (±5.55 ms) 879.45 (±6.50 ms) -14.0ms (1.6%) 👍
table-create-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table/create/1k duration 117.00 (±3.00 ms) 117.50 (±3.65 ms) +0.5ms (0.4%) 👌
table-update-10th-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table/update-10th/1k duration 77.05 (±6.10 ms) 72.30 (±2.50 ms) -4.8ms (6.2%) 👌
tablecmp-append-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-component/append/1k duration 221.90 (±8.85 ms) 225.70 (±11.00 ms) +3.8ms (1.7%) 👌
tablecmp-clear-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-component/clear/1k duration 6.50 (±1.10 ms) 6.30 (±1.05 ms) -0.2ms (3.1%) 👌
tablecmp-create-10k metric base(74a42fc) target(1c2878b) trend
benchmark-table-component/create/10k duration 1713.80 (±15.35 ms) 1748.15 (±12.80 ms) +34.3ms (2.0%) 👎
tablecmp-create-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-component/create/1k duration 206.95 (±3.65 ms) 208.25 (±4.90 ms) +1.3ms (0.6%) 👌
tablecmp-update-10th-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-component/update-10th/1k duration 70.15 (±4.75 ms) 69.25 (±4.45 ms) -0.9ms (1.3%) 👌
wc-append-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-wc/append/1k duration 230.70 (±17.30 ms) 231.90 (±20.15 ms) +1.2ms (0.5%) 👌
wc-clear-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-wc/clear/1k duration 11.20 (±1.50 ms) 11.50 (±2.10 ms) +0.3ms (2.7%) 👌
wc-create-10k metric base(74a42fc) target(1c2878b) trend
benchmark-table-wc/create/10k duration 1857.95 (±21.50 ms) 1880.95 (±16.60 ms) +23.0ms (1.2%) 👎
wc-create-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-wc/create/1k duration 210.45 (±7.00 ms) 214.90 (±5.90 ms) +4.5ms (2.1%) 👎
wc-update-10th-1k metric base(74a42fc) target(1c2878b) trend
benchmark-table-wc/update-10th/1k duration 68.75 (±4.70 ms) 68.45 (±5.10 ms) -0.3ms (0.4%) 👌

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.

Move this flag to the BABEL_CONFIG_BASE from (the import is ../babel-plugins)

Copy link
Member

@pmdartus pmdartus left a comment

Choose a reason for hiding this comment

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

Except for @diervo comment, it looks good to merge.

@salesforce-best-lwc-internal
Copy link

Benchmark results

Base commit: a257c23 | Target commit: e77e8ae

lwc-engine-benchmark

table-append-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table/append/1k duration 146.70 (±3.55 ms) 146.50 (±4.65 ms) -0.2ms (0.1%) 👌
table-clear-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table/clear/1k duration 11.25 (±0.60 ms) 10.15 (±0.55 ms) -1.1ms (9.8%) 👍
table-create-10k metric base(a257c23) target(e77e8ae) trend
benchmark-table/create/10k duration 875.80 (±7.45 ms) 874.05 (±7.35 ms) -1.8ms (0.2%) 👌
table-create-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table/create/1k duration 118.45 (±2.80 ms) 117.00 (±2.60 ms) -1.5ms (1.2%) 👌
table-update-10th-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table/update-10th/1k duration 75.70 (±3.05 ms) 76.45 (±5.50 ms) +0.8ms (1.0%) 👌
tablecmp-append-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-component/append/1k duration 224.65 (±11.55 ms) 217.45 (±7.60 ms) -7.2ms (3.2%) 👍
tablecmp-clear-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-component/clear/1k duration 6.45 (±1.25 ms) 5.70 (±0.90 ms) -0.8ms (11.6%) 👌
tablecmp-create-10k metric base(a257c23) target(e77e8ae) trend
benchmark-table-component/create/10k duration 1725.80 (±13.10 ms) 1701.05 (±7.25 ms) -24.8ms (1.4%) 👍
tablecmp-create-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-component/create/1k duration 210.70 (±4.25 ms) 205.10 (±5.95 ms) -5.6ms (2.7%) 👍
tablecmp-update-10th-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-component/update-10th/1k duration 64.95 (±3.85 ms) 66.00 (±4.90 ms) +1.1ms (1.6%) 👌
wc-append-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-wc/append/1k duration 224.65 (±15.15 ms) 227.25 (±16.70 ms) +2.6ms (1.2%) 👌
wc-clear-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-wc/clear/1k duration 11.05 (±1.40 ms) 10.50 (±1.30 ms) -0.6ms (5.0%) 👌
wc-create-10k metric base(a257c23) target(e77e8ae) trend
benchmark-table-wc/create/10k duration 1820.10 (±21.50 ms) 1856.50 (±13.65 ms) +36.4ms (2.0%) 👎
wc-create-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-wc/create/1k duration 213.30 (±6.45 ms) 214.35 (±6.30 ms) +1.1ms (0.5%) 👌
wc-update-10th-1k metric base(a257c23) target(e77e8ae) trend
benchmark-table-wc/update-10th/1k duration 66.10 (±4.10 ms) 67.15 (±3.85 ms) +1.1ms (1.6%) 👌

@diervo diervo merged commit 1477d56 into master Apr 27, 2019
@diervo diervo deleted the midzelis/babelconfig branch April 27, 2019 02:12
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