-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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(scanner): respect experimentalDecorators: true
#15206
fix(scanner): respect experimentalDecorators: true
#15206
Conversation
Run & review this pull request in StackBlitz Codeflow. |
// make esbuild handle js/ts/jsx/tsx | ||
// so that tsconfig.json is respected by esbuild |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this mean that we let esbuild resolve instead? Wouldn't that possibly diverge from Vite's resolver that we want to use instead?
Maybe we can fix this issue by enabling Looks like I explicitly removed experimentalDecorators
by default only for scanningexperimentalDecorators
support in scanning. Hmm I'm confused though why esbuild doesn't respect the tsconfig if we manually resolve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm true.
It seems esbuild doesn't support reading tsconfig.json
if the plugin resolved the path (evanw/esbuild#2265).
related code: the part resolving tsconfig, resolver (res.Resolve
calls finalizeResolve
which resolves tsconfig)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As much as I'd like to avoid it, maybe reverting and enabling back experimentalDecorators
for scanning would be the easier way for now 🤔 I think it shouldn't hurt too much since we only need to scan things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading microsoft/TypeScript#50820, it seems there's some incompatibilities between the syntax of them unfortunately. 😢
Example 1 .(private fields), Example 2 (class expressions) (the error disappears if experimentalDecorators
is set to false
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dang, perhaps what we can do for now is manually read the project's tsconfig.json
and then infer experimentalDecorators
from there? Seems like there will always be a tradeoff, but worst case they could manually disable/enable it with esbuildOptions.tsconfigRaw
perhaps.
It's been a while, but I changed it this way (#15206 (comment)). |
/ecosystem-ci run |
📝 Ran ecosystem CI on
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating!
Description
I totally forgot about #13805 (comment).
fixes #15201
refs #14804
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).