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(rollup-plugin): emit warnings during compilation #2833

Merged
merged 2 commits into from
May 16, 2022

Conversation

nolanlawson
Copy link
Collaborator

Details

Fixes #2771

Propagates template compiler warnings from @lwc/compiler to @lwc/rollup-plugin, where it is logged using the standard Rollup this.warn() method as a RollupWarning.

Here is what a warning looks like when running rollup in the console (source:

(!) Plugin rollup-plugin-lwc-compiler: @lwc/rollup-plugin: LWC1148: Invalid HTML syntax: end-tag-without-matching-open-element. This will not be supported in future versions of LWC. For more information, please visit https://html.spec.whatwg.org/multipage/parsing.html#parse-error-end-tag-without-matching-open-element
src/modules/x/app/app.html: (4:1)
</template>

Screenshot:

Screen Shot 2022-05-12 at 2 07 57 PM

Does this pull request introduce a breaking change?

  • ✅ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • ⚠️ Yes, it does include an observable change.

If you're using the Rollup plugin directly (e.g. off-core), and if you were using onwarn to capture warnings, it previously would never be called. Now it will be called with a RollupWarning.

GUS work item

W-10930894

@@ -24,7 +24,8 @@
],
"devDependencies": {
"@lwc/compiler": "2.13.3",
"@lwc/engine-dom": "2.13.3"
"@lwc/engine-dom": "2.13.3",
"@lwc/errors": "2.13.3"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

devDep because we're just doing import type.

@@ -17,6 +22,7 @@ import scriptTransformer from './javascript';
export interface TransformResult {
code: string;
map: unknown;
warnings?: CompilerDiagnostic[];
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The style and babel compilers don't have warnings propagated here, although I suppose we could do that in the future.

@nolanlawson nolanlawson requested review from rui-rayqiu and jmsjtu May 12, 2022 22:48
@@ -42,6 +42,7 @@ const { code } = await transform(source, filename, options);

- `code` (string) - the compiled source code.
- `map` (object) - the generated source map.
- `warnings` (array, optional) - the array of diagnostic warnings, if any.
Copy link
Member

Choose a reason for hiding this comment

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

Why not return all the diagnostics instead of just the warnings and the consumer decide what to do with the diagnostics?

Copy link
Contributor

@ravijayaramappa ravijayaramappa May 16, 2022

Choose a reason for hiding this comment

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

Looking at the oss compiler, all Error diagnostics cause an exception. There are no Log and Fatal level Diagnostics. That effectively leaves only Warning level diagnostics that can be present in this output.

This declaration makes an implicit behavior, explicit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct. I can add a comment to this effect.

Comment on lines +88 to +91
// The CompilerDiagnostic from @lwc/template-compiler reports an undefined filename, because it loses the
// filename context here:
// https://github.com/salesforce/lwc/blob/e2bc36f/packages/%40lwc/compiler/src/transformers/template.ts#L35-L38
file: id,
Copy link
Member

Choose a reason for hiding this comment

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

It would be a nice improvement to pass the file name to the template compiler to produce more meaningful diagnostics.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, but I left it out of this PR. We can always do it in the future, and the test verifies that the file property is correct.

@nolanlawson nolanlawson merged commit e92881c into master May 16, 2022
@nolanlawson nolanlawson deleted the nolan/rollup-warnings branch May 16, 2022 19:21
caridy pushed a commit that referenced this pull request Jun 7, 2022
wip: add a static vnode type

wip: needs key and patch

wip: all tests passing exept a few because of simple things

wip: set style tokens for fragments during template evaluation

wip: set element shadow token for fragments during template evaluation

wip: propagate the shadow resolver key of static fragments

wip: do not gen static nodes for text or comments

wip: use tagedTemplate expression to replate stylesheetToken

wip: use cloneNode from renderer

wip: treeWalker to work in ie11

refactor: do not strip empty attr or empty class attr

fix: using incorrect key

wip: trim value of textNodes and review feedback

fix: hydration

feat: custom static element serializer

wip: remove unessesary import

fix: hydration

fix: snapshot tests

fix: missing karma test

fix: test due rebase

test: add test for static content needing nodeOwner

fix: escape strings in serializer

refactor: remove unused apis on generated code

refactor: review suggestions

fix: support mixed mode

wip: fix compilation snapshots

fix: increase 0.5kb bundlesize for engine dom

fix: flapper

wip: helpers.ts review

wip: codegen.ts review

wip: missing items from pm review

wip: review comments

fix: respect preserveComments and fuse $1,2 into 3

fix: svg content with the correct namespace

feat(template-compiler): add option to disable static content optimizations

wip: remove invalid comment

chore: bump version to v2.13.0 (#2784)

chore: dependencies upgrade (#2785)

test: fix Node warning about event emitters (#2789)

chore: run karma and integration tests in parallel (#2792)

* chore: run karma and integration tests in parallel

* fix: remove log lines

fix(babel-plugin-component): remove import validation (#2719)

test: remove flakey IE integration test (#2796)

test: update test to use lwc imports (#2794)

chore: Restrict further import order (#2795)

chore: bump version to v2.13.1 (#2804)

refactor(engine): moving vm references from dom into core (#2801)

* refactor(engine): moving vm references from dom into core

chore(nucleus): remove salesforcedevs/developer-website (#2807)

test(integration-karma): small quality-of-life improvements (#2809)

chore(deps): bump ejs from 3.1.6 to 3.1.7 (#2810)

chore: weekly dependencies upgrade (#2816)

* chore: weekly dependencies upgrade

* fix: update yarn.lock`

refactor(engine): optimize computation of transitive shadow mode (#2803)

chore(deps): bump async from 2.6.3 to 2.6.4 (#2815)

Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4.
- [Release notes](https://github.com/caolan/async/releases)
- [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md)
- [Commits](caolan/async@v2.6.3...v2.6.4)

---
updated-dependencies:
- dependency-name: async
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

chore: bump version to v2.13.2 (#2819)

chore: retry failed Circle CI tests (#2814)

* chore: retry failed Circle CI tests

W-10946477

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: deliberately fail a test to see what happens

* chore: improve retry script

* fix: whitespace

* Revert "chore: deliberately fail a test to see what happens"

This reverts commit 611fc34.

* chore: rename to retry_karma

fix(engine-core): add shim for old stylesheetTokens internal API (#2821)

W-11093934

chore: bump version to v2.13.3 (#2823)

fix(build): remove swc, switch back to babel and terser (#2818)

feat: add freezeTemplate() API, warn on mutation (#2825)

* feat: add freezeTemplate() API, warn on mutation

* fix: warn on slots/renderMode as well

* fix: add comment

* fix: remove duplicate process.env.NODE_ENV check

fix(engine-dom): refactor stylesheet API for consistency (#2827)

* fix(engine-dom): refactor stylesheet API for consistency

* fix: remove useless code comment

* test: remove unnecessary test

* test: remove unnecessary test

* refactor: slight refactor

* fix: add code comments

* fix: add code comments

* fix: add better comment

fix: relax static id validation in iterations (#2830)

fix(rollup-plugin): emit warnings during compilation (#2833)

* fix(rollup-plugin): emit warnings during compilation

Fixes #2771

W-10930894

* fix: add code comment

fix(engine-dom): make feature flags work (#2812)

* fix(engine-dom): make feature flags work

Fixes #2811

* fix: license headers

* test: fix jest tests

* test: fix test

* test: fix test

* fix: use Eugene's technique instead

* Revert "fix: use Eugene's technique instead"

This reverts commit 72afdc0.

* fix: use Eugene's technique instead

* fix: revert unnecessary test change

* fix: revert, use the elaborate test instead

* fix: fix feature flags in engine-server as well

perf(engine-dom): refactor style cache to reduce lookups (#2832)

* perf(engine-dom): refactor style cache to reduce lookups

* fix: tidy up comments

* fix: update packages/@lwc/engine-dom/src/styles.ts

Co-authored-by: Pierre-Marie Dartus <p.dartus@salesforce.com>

* fix: remove semi

* fix: remove "used" flag

* fix: refactor

* fix: refactor

* fix: bring back "used" flag

* fix: typo

Co-authored-by: Pierre-Marie Dartus <p.dartus@salesforce.com>

chore: update deps (#2838)

test: run feature flag test code only in karma (#2835)

fix: trigger slotchange event on removing slot (#2840)

test(integration-karma): silence lwc rollup plugin warnings (#2836)

* test(integration-karma): silence lwc rollup plugin warnings

* fix: use warn API

v2.11.7 (#2842)

chore: release v2.14.0 (#2846)

fix: only remove slot children in synthetic shadow (#2843)

* fix: only remove slot children in synthetic shadow

* fix: use case block

fix: only add version mismatch test code in karma (#2852)

test(integration-karma): ensure constructable stylesheets are re-used (#2844)

* test(integration-karma): ensure constructable stylesheets are re-used

* test: add test for shared style

chore(nucleus): remove more downstreams (#2855)

chore(nucleus): remove another downstream (#2857)

docs: fix typo in template compiler readme (#2848)

* docs: fix typo in template compiler readme

* docs: rewording usage of lwc dynamic directive

Co-authored-by: Eugene Kashida <ekashida@gmail.com>

Co-authored-by: Eugene Kashida <ekashida@gmail.com>

chore: fix lint

test: refactor test, remove test covered in #2859

test: on second thought, bring test back
nolanlawson pushed a commit that referenced this pull request Jun 7, 2022
wip: add a static vnode type

wip: needs key and patch

wip: all tests passing exept a few because of simple things

wip: set style tokens for fragments during template evaluation

wip: set element shadow token for fragments during template evaluation

wip: propagate the shadow resolver key of static fragments

wip: do not gen static nodes for text or comments

wip: use tagedTemplate expression to replate stylesheetToken

wip: use cloneNode from renderer

wip: treeWalker to work in ie11

refactor: do not strip empty attr or empty class attr

fix: using incorrect key

wip: trim value of textNodes and review feedback

fix: hydration

feat: custom static element serializer

wip: remove unessesary import

fix: hydration

fix: snapshot tests

fix: missing karma test

fix: test due rebase

test: add test for static content needing nodeOwner

fix: escape strings in serializer

refactor: remove unused apis on generated code

refactor: review suggestions

fix: support mixed mode

wip: fix compilation snapshots

fix: increase 0.5kb bundlesize for engine dom

fix: flapper

wip: helpers.ts review

wip: codegen.ts review

wip: missing items from pm review

wip: review comments

fix: respect preserveComments and fuse $1,2 into 3

fix: svg content with the correct namespace

feat(template-compiler): add option to disable static content optimizations

wip: remove invalid comment

chore: bump version to v2.13.0 (#2784)

chore: dependencies upgrade (#2785)

test: fix Node warning about event emitters (#2789)

chore: run karma and integration tests in parallel (#2792)

* chore: run karma and integration tests in parallel

* fix: remove log lines

fix(babel-plugin-component): remove import validation (#2719)

test: remove flakey IE integration test (#2796)

test: update test to use lwc imports (#2794)

chore: Restrict further import order (#2795)

chore: bump version to v2.13.1 (#2804)

refactor(engine): moving vm references from dom into core (#2801)

* refactor(engine): moving vm references from dom into core

chore(nucleus): remove salesforcedevs/developer-website (#2807)

test(integration-karma): small quality-of-life improvements (#2809)

chore(deps): bump ejs from 3.1.6 to 3.1.7 (#2810)

chore: weekly dependencies upgrade (#2816)

* chore: weekly dependencies upgrade

* fix: update yarn.lock`

refactor(engine): optimize computation of transitive shadow mode (#2803)

chore(deps): bump async from 2.6.3 to 2.6.4 (#2815)

Bumps [async](https://github.com/caolan/async) from 2.6.3 to 2.6.4.
- [Release notes](https://github.com/caolan/async/releases)
- [Changelog](https://github.com/caolan/async/blob/v2.6.4/CHANGELOG.md)
- [Commits](caolan/async@v2.6.3...v2.6.4)

---
updated-dependencies:
- dependency-name: async
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

chore: bump version to v2.13.2 (#2819)

chore: retry failed Circle CI tests (#2814)

* chore: retry failed Circle CI tests

W-10946477

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: fix

* chore: deliberately fail a test to see what happens

* chore: improve retry script

* fix: whitespace

* Revert "chore: deliberately fail a test to see what happens"

This reverts commit 611fc34.

* chore: rename to retry_karma

fix(engine-core): add shim for old stylesheetTokens internal API (#2821)

W-11093934

chore: bump version to v2.13.3 (#2823)

fix(build): remove swc, switch back to babel and terser (#2818)

feat: add freezeTemplate() API, warn on mutation (#2825)

* feat: add freezeTemplate() API, warn on mutation

* fix: warn on slots/renderMode as well

* fix: add comment

* fix: remove duplicate process.env.NODE_ENV check

fix(engine-dom): refactor stylesheet API for consistency (#2827)

* fix(engine-dom): refactor stylesheet API for consistency

* fix: remove useless code comment

* test: remove unnecessary test

* test: remove unnecessary test

* refactor: slight refactor

* fix: add code comments

* fix: add code comments

* fix: add better comment

fix: relax static id validation in iterations (#2830)

fix(rollup-plugin): emit warnings during compilation (#2833)

* fix(rollup-plugin): emit warnings during compilation

Fixes #2771

W-10930894

* fix: add code comment

fix(engine-dom): make feature flags work (#2812)

* fix(engine-dom): make feature flags work

Fixes #2811

* fix: license headers

* test: fix jest tests

* test: fix test

* test: fix test

* fix: use Eugene's technique instead

* Revert "fix: use Eugene's technique instead"

This reverts commit 72afdc0.

* fix: use Eugene's technique instead

* fix: revert unnecessary test change

* fix: revert, use the elaborate test instead

* fix: fix feature flags in engine-server as well

perf(engine-dom): refactor style cache to reduce lookups (#2832)

* perf(engine-dom): refactor style cache to reduce lookups

* fix: tidy up comments

* fix: update packages/@lwc/engine-dom/src/styles.ts

Co-authored-by: Pierre-Marie Dartus <p.dartus@salesforce.com>

* fix: remove semi

* fix: remove "used" flag

* fix: refactor

* fix: refactor

* fix: bring back "used" flag

* fix: typo

Co-authored-by: Pierre-Marie Dartus <p.dartus@salesforce.com>

chore: update deps (#2838)

test: run feature flag test code only in karma (#2835)

fix: trigger slotchange event on removing slot (#2840)

test(integration-karma): silence lwc rollup plugin warnings (#2836)

* test(integration-karma): silence lwc rollup plugin warnings

* fix: use warn API

v2.11.7 (#2842)

chore: release v2.14.0 (#2846)

fix: only remove slot children in synthetic shadow (#2843)

* fix: only remove slot children in synthetic shadow

* fix: use case block

fix: only add version mismatch test code in karma (#2852)

test(integration-karma): ensure constructable stylesheets are re-used (#2844)

* test(integration-karma): ensure constructable stylesheets are re-used

* test: add test for shared style

chore(nucleus): remove more downstreams (#2855)

chore(nucleus): remove another downstream (#2857)

docs: fix typo in template compiler readme (#2848)

* docs: fix typo in template compiler readme

* docs: rewording usage of lwc dynamic directive

Co-authored-by: Eugene Kashida <ekashida@gmail.com>

Co-authored-by: Eugene Kashida <ekashida@gmail.com>

chore: fix lint

test: refactor test, remove test covered in #2859

test: on second thought, bring test back
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.

Rollup plugin does not warn on parser warnings
3 participants