lwc icon indicating copy to clipboard operation
lwc copied to clipboard

fix: remove word boundaries from decorator check regex

Open yashank676 opened this issue 2 months ago • 13 comments

Details

/\b(track|api|wire)\b/ tests false if track is with ANSI escape codes for color output in terminals e.g. \033[31;1;4mtrack\033[0m

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.
  • The user will now get LWC1198 error instead of LWC1007 for Babel decorator error consistently.

yashank676 avatar Oct 09 '25 14:10 yashank676

Thanks for the contribution! It looks like @yashank-aggarwal_sfemu is an internal user so signing the CLA is not required. However, we need to confirm this.

salesforce-cla[bot] avatar Oct 09 '25 14:10 salesforce-cla[bot]

What is the motivation for this change? Please also add some test cases to demonstrate the changed behavior.

wjhsf avatar Oct 09 '25 14:10 wjhsf

@wjhsf The regex condition present was not able to search for the decorator due to ANSI format of the error code given to match, in cases like eg it was able to match the regex due to import statement present and hence threw 1198 error but if we have anything else added after import statement (try adding a comment eg), the regex wont be able to match and 1007 error would be thrown which caused inconsistency.

yashank676 avatar Oct 10 '25 09:10 yashank676

When the LWC compiler encounters a babel error, it checks the message for the LWC decorators (api/track/wire), and provides a more helpful message if encountered (LWC1198, rather than LWC1007). However, it checks using regex word boundaries (\b), and babel uses ANSI escape codes (\x1b[39m) for formatting the how the error message is printed to console, causing the regex to sometimes report false, even though an LWC decorator is used. For example, \x1b[33m@\x1b[39mapi prints @api with a yellow @. This is a scenario that we should capture, but do not.

The actual decorator usage that causes the error seems to always be preceded by a color token. However, babel includes a few lines of code as context prior to the erroneous decorator usage. This means that whether we get LWC1198 or LWC1007 is dependent on that context. For example, if the context includes import { api } from 'lwc', then we will get LWC1198. If not, we will get LWC1007. This behavior is inconsistent and confusing.


Consider this class, which is not a component:

import { eschatology } from 'vermont'
export default class Bananaphone {
  @eschatology biggens
}

With the proposed change, the above code will fail to compile with "Error: SyntaxError: LWC1198: Decorators like @api, @track, and @wire are only supported in LightningElement classes." That's misleading, because neither the decorators nor LightningElement are involved in this scenario.

A better fix, here, would be to update the regex to account for the possibility of ANSI escape codes, so that we still only add the LWC context when actually relevant. We should not rely on any specific codes being present, as babel may change the color of their output at any time.

wjhsf avatar Oct 10 '25 17:10 wjhsf

Agreed to all the points, i am already looking into it and see if some regex can be made to handle that.

yashank676 avatar Oct 10 '25 17:10 yashank676

@wjhsf i have updated the regex. Also, looks like updating unit test for this makes no sense as the issue we faced in previous regex was due to ANSI format in error, but in vitest it was a simple string error instead.

yashank676 avatar Oct 14 '25 15:10 yashank676

Thanks for the contribution! Before we can merge this, we need @yashank676 to sign the Salesforce Inc. Contributor License Agreement.

salesforce-cla[bot] avatar Oct 14 '25 15:10 salesforce-cla[bot]

@yashank676 please make sure the CLA is signed and add two test cases to packages/@lwc/compiler/src/transformers/__tests__/transform-javascript.spec.ts.

wjhsf avatar Oct 14 '25 16:10 wjhsf

@wjhsf i have already signed the CLA dont know why its not showing it here. For test cases do you mean cases that i just mentioned above?

  1. import { eschatology } from 'vermont' export default class Bananaphone { @eschatology prewire }
  2. import { eschatology } from 'vermont' export default class Bananaphone { @eschatology wire }

yashank676 avatar Oct 16 '25 18:10 yashank676

I think these test cases cover the different possible scenarios, but feel free to come up with your own.

// Using an LWC decorator -- no comment
import { api } from 'lwc'
export default class NotComponent {
  @api prop
}

// Using an LWC decorator -- with comment
import { api } from 'lwc'
// the error message should not include the above import
export default class NotComponent {
  @api prop
}

// Using an almost-LWC decorator
const apitrackwire = () => {}
export default class NotComponent {
  @apitrackwire prop
}

// Using a non-LWC decorator
const decorator = () => {}
export default class NotComponent {
  @decorator prop
}

wjhsf avatar Oct 16 '25 18:10 wjhsf

should we be adding test cases like:

const apitrackwire = () => {}
export default class NotComponent {
  @apitrackwire prop
}

since ideally they should throw 1007 but since we can't right such stricter regex, it throws 1198 for either regex you mentioned or a simpler one (still confused which one we should use)

yashank676 avatar Oct 16 '25 18:10 yashank676

Include whatever test cases you think are relevant, and have them assert the correct error for the regex used.

wjhsf avatar Oct 16 '25 19:10 wjhsf

/nucleus test

wjhsf avatar Oct 17 '25 17:10 wjhsf