Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: remove optional chaining from code that will be transpiled #3698

Conversation

achingbrain
Copy link
Member

Optional chaining is supported in node and in browsers but tools that
transpile code to run in older browsers doesn't understand the syntax.

Specifically the version of acorn that at least vue uses via a dependency
on webpack 4 does not support this.

It'll be fixed by them upgrading to webpack 5 but the timescale on that
is unclear, which is why we're using a prerelease version in our examples.

The short term fix is to just remove optional chaining from where we
are using it in code that will be transpiled for browser use.

Optional chaining is supported in node and in browsers but tools that
transpile code to run in older browsers doesn't understand the syntax.

Specifically the version of acorn that at least vue uses via a dependency
on webpack 4 does not support this.

It'll be fixed by them upgrading to webpack 5 but the timescale on that
is unclear, which is why we're using a prerelease version in our examples.

The short term fix is to just remove optional chaining from where we
are using it in code that will be transpiled for browser use.
@achingbrain achingbrain requested a review from lidel May 25, 2021 14:24
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

👍 for short term fix, but this type of thing often comes back as new PRs get merged.

Should we enforce https://eslint.org/docs/rules/no-unsafe-optional-chaining check at ci/eslint/aegir level for now? (cc @hugomrdias)

@achingbrain
Copy link
Member Author

I don't think that rule would do it, we'd want to disallow optional chaining altogether rather than just when the thing we think the property of is optional is undefined itself.

Copy link
Member

@hugomrdias hugomrdias left a comment

Choose a reason for hiding this comment

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

i agree with alex, we should avoid optional chaining for now and i dont think there a eslint rule for that

@achingbrain achingbrain merged commit 96b3909 into master May 25, 2021
@achingbrain achingbrain deleted the fix/remove-optional-chaining-from-code-that-will-be-transpiled branch May 25, 2021 15:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants