feat(compiler): migrate referential integrity into lwc-compiler #109

apapko · 2018-02-14T23:06:23Z

Details

Migrate referential integrity report from lwc-platform compiler into salesforce/lwc compiler.
Fixed the following work item, which is part of the LWC Deploy/Retrieve Epic

Does this PR introduce a breaking change?

Yes
No

davidturissini · 2018-02-14T23:14:28Z

packages/lwc-compiler/tsconfig.json

+    "module": "es2015",
+    "sourceMap": true,
+    // TODO: turn back to true when there are no more js files in the package
+    // "declaration": true,


Comments are not valid JSON. One way to do this would be to add a // entry instead:

{ "sourceMap": true, "//": "TODO: turn back to true when there are no more js files in the package" "declaration": true, }

Ahah. I didn't know this trick, it's awesome!
That being said tsc use json5 to parse the configuration, adding comments is fine.

davidturissini · 2018-02-14T23:15:24Z

packages/lwc-compiler/tsconfig.json

+
+    // Enhance Strictness
+    "strict": true,
+    "suppressImplicitAnyIndexErrors": true


Add noUnusedLocals rule (https://www.typescriptlang.org/docs/handbook/compiler-options.html)

pmdartus · 2018-02-14T23:09:49Z

packages/lwc-compiler/package.json

@@ -7,16 +7,19 @@
  "license": "ISC",
  "scripts": {
    "clean": "rimraf dist/",
-    "build": "babel src/ --out-dir dist/commonjs",
-    "build:umd": "webpack --progress",


We will probably need to keep the build:umd for now until we update the delivery mechanism of the compiler into Aura.

pmdartus · 2018-02-14T23:11:18Z

packages/lwc-compiler/src/__tests__/references/css.spec.ts

@@ -0,0 +1,342 @@
+import { getReferences } from '../../references/css';


Collocate the test file closer to the actual implementation in packages/lwc-compiler/src/references/__tests__/css.spec.ts

pmdartus · 2018-02-14T23:12:00Z

packages/lwc-compiler/src/__tests__/references/html.spec.ts

@@ -0,0 +1,78 @@
+import { getReferences } from '../../references/html';


Same comment than for the css.spec.ts

pmdartus · 2018-02-14T23:12:09Z

packages/lwc-compiler/src/__tests__/references/javascript.spec.ts

@@ -0,0 +1,115 @@
+import { getReferences } from '../../references/javascript';


Same comment than for the css.spec.ts

pmdartus · 2018-02-14T23:13:29Z

packages/lwc-compiler/src/references/references.ts

+import { LwcBundle } from '../lwc-bundle';
+
+export function getBundleReferences(bundle: LwcBundle): Reference[] {
+    console.log("BUNDLE", bundle);


Remove console.log

pmdartus · 2018-02-14T23:19:48Z

packages/lwc-compiler/package.json

    "cssnano": "^3.10.0",
    "lwc-template-compiler": "0.17.15",
+    "parse5": "^3.0.3",


Could you make sure the versions of the packages align with the other packages?
For example ts-jest is not in sync with the other package, and it causes new entries to be added in the yarn.lock

pmdartus · 2018-02-15T01:10:46Z

packages/lwc-engine/package.json

@@ -24,7 +24,7 @@
    "rollup-plugin-replace": "^2.0.0",
    "rollup-plugin-typescript": "^0.8.1",
    "rollup-plugin-uglify": "3.0.0",
-    "ts-jest": "^21.0.1",
+    "ts-jest": "^22.0.4",


This PR should not update the version of ts-jest in the other packages. We should update that in another PR, there are several packages that need to be updated along with ts-jest (eg. @types/jest, jest).

Let's keep the PR scoped to lwc-compiler, and set the ts-jest to ^21.0.1 to be consistent with the other packages.

Using 21.2.4 as discovered during the debugging with @pmdartus

pmdartus · 2018-02-15T17:53:17Z

packages/lwc-compiler/package.json

    "build:umd": "webpack --progress",
    "test": "jest --coverage --maxWorkers=2"
  },
  "devDependencies": {
    "babel-cli": "^6.24.0",
+    "babel-jest": "^22.2.2",


Remove babel-jest

salesforce-best-lwc-internal · 2018-02-15T18:48:48Z

Benchmark comparison

Base commit: 304389b | Target commit: 5373a3d

benchmark	base(`304389b`)	target(`5373a3d`)	trend
table-append-1k.benchmark:benchmark-table/append/1k	250.38 (± 4.04 ms)	255.47 (± 3.67 ms)	👎
table-clear-1k.benchmark:benchmark-table/clear/1k	12.68 (± 0.51 ms)	13.13 (± 0.59 ms)	👎
table-create-10k.benchmark:benchmark-table/create/10k	1406.72 (± 12.34 ms)	1430.47 (± 15.57 ms)	👎
table-create-1k.benchmark:benchmark-table/create/1k	149.13 (± 3.01 ms)	149.82 (± 2.21 ms)	👌
table-update-10th-1k.benchmark:benchmark-table/update-10th/1k	140.34 (± 2.52 ms)	145.81 (± 3.76 ms)	👎
tablecmp-append-1k.benchmark:benchmark-table-component/append/1k	322.15 (± 7.03 ms)	321.63 (± 13.04 ms)	👌
tablecmp-clear-1k.benchmark:benchmark-table/clear/1k	33.04 (± 1.16 ms)	32.99 (± 1.45 ms)	👌
tablecmp-create-10k.benchmark:benchmark-table-component/create/10k	2049.21 (± 30.74 ms)	2038.36 (± 20.54 ms)	👌
tablecmp-create-1k.benchmark:benchmark-table-component/create/1k	225.29 (± 5.61 ms)	232.47 (± 4.72 ms)	👎
tablecmp-update-10th-1k.benchmark:benchmark-table-component/update-10th/1k	133.97 (± 5.60 ms)	137.96 (± 4.30 ms)	👎

salesforce-best-lwc-internal · 2018-02-15T22:47:11Z

Benchmark comparison

Base commit: 17d0379 | Target commit: 63382ee

benchmark	base(`17d0379`)	target(`63382ee`)	trend
table-append-1k.benchmark:benchmark-table/append/1k	244.21 (± 5.50 ms)	258.68 (± 8.79 ms)	👎
table-clear-1k.benchmark:benchmark-table/clear/1k	12.63 (± 0.52 ms)	13.47 (± 0.56 ms)	👎
table-create-10k.benchmark:benchmark-table/create/10k	1430.13 (± 34.65 ms)	1505.48 (± 43.33 ms)	👎
table-create-1k.benchmark:benchmark-table/create/1k	148.22 (± 1.70 ms)	153.67 (± 2.88 ms)	👎
table-update-10th-1k.benchmark:benchmark-table/update-10th/1k	138.31 (± 2.29 ms)	143.27 (± 2.84 ms)	👎
tablecmp-append-1k.benchmark:benchmark-table-component/append/1k	317.62 (± 5.89 ms)	324.73 (± 10.54 ms)	👎
tablecmp-clear-1k.benchmark:benchmark-table/clear/1k	29.74 (± 1.18 ms)	30.52 (± 1.18 ms)	👎
tablecmp-create-10k.benchmark:benchmark-table-component/create/10k	1987.46 (± 28.47 ms)	2027.66 (± 28.86 ms)	👎
tablecmp-create-1k.benchmark:benchmark-table-component/create/1k	222.42 (± 1.98 ms)	234.55 (± 6.21 ms)	👎
tablecmp-update-10th-1k.benchmark:benchmark-table-component/update-10th/1k	132.26 (± 3.55 ms)	144.26 (± 5.63 ms)	👎

caridy · 2018-02-16T05:06:39Z

packages/lwc-compiler/src/references/javascript.ts

+    ];
+}
+
+function sfdcReferencesVisitor(references: Reference[], filename: string) {


why do we have sfdc references in this pkg?

I thought this was going to be generic enough that can be used by sfdc and others.

caridy · 2018-02-16T05:08:06Z

packages/lwc-compiler/src/references/types.ts

@@ -0,0 +1,11 @@
+export declare type ReferenceType = 'resourceUrl' | 'label' | 'apexClass' | 'apexMethod' | 'sobjectClass' | 'sobjectField' | 'component';


why does the compiler care about apex or resources or sobjects?

diervo · 2018-02-16T06:55:19Z

This is temporal as we move things around. We need to update you with the latest discussions around the platform compiler

…

On Thu, Feb 15, 2018 at 9:08 PM Caridy Patiño ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In packages/lwc-compiler/src/references/types.ts <#109 (comment)>: > @@ -0,0 +1,11 @@ +export declare type ReferenceType = 'resourceUrl' | 'label' | 'apexClass' | 'apexMethod' | 'sobjectClass' | 'sobjectField' | 'component'; why does the compiler care about apex or resources or sobjects? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#109 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABH0qI3yLfZOzUe9qUg5ptz3PeNfxFBNks5tVQ02gaJpZM4SGHQB> .

apapko added 6 commits February 13, 2018 14:03

feat(packages): enable typescript

0a88b3f

feat(jest): run tests for both js and ts

732aec0

feat(jest): run tests for both js and ts

4d439ce

compiler-references

e9fc521

Merge branch 'master' into apapko/compiler-references

1518d2e

wip: remove unused import and stale comment

1d2e404

apapko requested review from diervo and pmdartus February 14, 2018 23:06

davidturissini reviewed Feb 14, 2018

View reviewed changes

pmdartus requested changes Feb 14, 2018

View reviewed changes

apapko added 2 commits February 14, 2018 16:52

wip: review comments

b037274

Merge branch 'master' into apapko/compiler-references

20bc151

pmdartus requested changes Feb 15, 2018

View reviewed changes

wip: change module imports and jest transform format

65aee92

pmdartus requested changes Feb 15, 2018

View reviewed changes

apapko added 2 commits February 15, 2018 09:56

wip: review comment - removed unused package

b02ac32

wip: correction to rollup-replace import

5373a3d

pmdartus approved these changes Feb 15, 2018

View reviewed changes

wip: merge conflict w master

63382ee

apapko merged commit 029e879 into master Feb 15, 2018

apapko deleted the apapko/compiler-references branch February 16, 2018 00:20

caridy reviewed Feb 16, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(compiler): migrate referential integrity into lwc-compiler #109

feat(compiler): migrate referential integrity into lwc-compiler #109

apapko commented Feb 14, 2018

davidturissini Feb 14, 2018

pmdartus Feb 14, 2018

davidturissini Feb 14, 2018

pmdartus Feb 14, 2018

pmdartus Feb 14, 2018

pmdartus Feb 14, 2018

pmdartus Feb 14, 2018

pmdartus Feb 14, 2018

pmdartus Feb 14, 2018

pmdartus Feb 15, 2018

apapko Feb 15, 2018

pmdartus Feb 15, 2018

salesforce-best-lwc-internal bot commented Feb 15, 2018

salesforce-best-lwc-internal bot commented Feb 15, 2018

caridy Feb 16, 2018

caridy Feb 16, 2018

caridy Feb 16, 2018

diervo commented Feb 16, 2018 via email

		@@ -0,0 +1,342 @@
		import { getReferences } from '../../references/css';

		@@ -0,0 +1,78 @@
		import { getReferences } from '../../references/html';

		@@ -0,0 +1,115 @@
		import { getReferences } from '../../references/javascript';

		@@ -0,0 +1,11 @@
		export declare type ReferenceType = 'resourceUrl' \| 'label' \| 'apexClass' \| 'apexMethod' \| 'sobjectClass' \| 'sobjectField' \| 'component';

feat(compiler): migrate referential integrity into lwc-compiler #109

feat(compiler): migrate referential integrity into lwc-compiler #109

Conversation

apapko commented Feb 14, 2018

Details

Does this PR introduce a breaking change?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salesforce-best-lwc-internal bot commented Feb 15, 2018

Benchmark comparison

salesforce-best-lwc-internal bot commented Feb 15, 2018

Benchmark comparison

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diervo commented Feb 16, 2018 via email