aurelia / framework

The Aurelia 1 framework entry point, bringing together all the required sub-modules of Aurelia.
MIT License
11.76k stars 623 forks source link

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

Open mitchcapper opened 7 months ago

mitchcapper commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 3 months ago

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