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

using typescript targeting ES2022 (or newer likely) silently breaks @observable and @bindable (partially) #1010

Open
mitchcapper opened this issue Apr 25, 2024 · 5 comments

Comments

@mitchcapper
Copy link

I'm submitting a bug report
changing typescript from targeting ES2022 from ES2021 will silently break aurelia in some non-obvious ways. Any getters/setters done through prototyping are broken. Any getters/setters done afterworks succeed.

Simple view of the generated js and why its an issue: https://page.thepages.workers.dev/aurelia_cry

Simple repo from the default blank project: https://github.com/mitchcapper/aureliaV1_es2022_target_repo
and the single commit that differs from the empty project: 2e2b9c7d027574554a59a56834a8ee7c5b486ca1

change it from es2022 to es2021 and it is fixed. Note you must restart the webpack engine after the change it will not auto-pickup that.

Current behavior:
When targetting ES2022 things like observable decorators are broken

Expected/desired behavior:
Same as with ES2021 where they work without issue

I don't know if this effects V2. I changed the targeting when trying to get an old aurelia project to work with more modern/node/webpack so tried to version up everything. As many things seem to work when this bug is present it took a good while to do the RCA.

@bigopon
Copy link
Member

bigopon commented Apr 26, 2024

Thanks @mitchcapper , v2 is not affected by this change. For v1, some work will be required to tackle this, and at the moment, it's not certain yet when that will be, probably until after decorator spec has been finalised.

@mitchcapper
Copy link
Author

yeah id assume its not needed to fix in v1 but if its possible to detect and warn of such behavior would be a plus:)

@thomas-darling
Copy link
Sponsor

Just wanna say, I would really like to see this fixed in v1 🥺

We'd love to upgrade, but v2 is not an incremental change, and we really have no viable upgrade path for our largest projects, some of which have evolved over more than haft a decade. And we're actually ok with that, as long as v1 is maintained enough to remain functional - it really is a remarkably good framework, that has stood the test of time like no other.

For now, the workaround is to add "useDefineForClassFields": true in your tsconfig.json file - but that probably won't work forever, so we really need a proper fix, though I can understand the desire to wait for the spec to be finalized.

Just please don't forget about it 🥺

@bigopon
Copy link
Member

bigopon commented Jun 12, 2024

It's just as stated above, we kind of want to make sure the effort is as minimal required and spent as possible, even though the proposal seems to be in the final steps before stage 4 already.
Btw @thomas-darling , thanks for you and your company contribution over the years, I'm grateful for that so quite likely I personally won't forget it 🥺

@bigopon
Copy link
Member

bigopon commented Aug 10, 2024

fyi, we have backported resolve from v2 here aurelia/dependency-injection#228
After new versions of dependency injection and this package go out, application can alleviate/mitigate issues with @autoinject / parameter injection using resolve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants