embroider-build / addon-blueprint

Blueprint for v2-formatted Ember addons
MIT License
29 stars 27 forks source link

decorator-transforms breaks consumers of ios apps running safari <16.4 #275

Closed void-mAlex closed 6 months ago

void-mAlex commented 6 months ago

it's unrealistic to expect all users to have phones new enough to be able to receive an update that is less than one year old. today by default any v2 addon that updated the blueprint far enough will break any consumers of the addon as with little (to no as I've found) way to post transpile the addon files in the app once static blocks are introduced and keep runtime decorator implementation happy across all usages of it

tried combinations v2 addon using decorators + app using decorators + babel plugin transform (different class syntax) breaks at runtime v2 addon using decorators + app legacy decorator transform - fails to compile v2 addon using decorators + app babel proposal decorators plus class transform plugins - fails to compile

proposing we revert the default babel config of latest addon bp to use plugin-proposal-decorators, as it's the only one that I managed to get working on safari 15.4 for example which some of our users are stuck on because Apple decisions to stop supporting those devices

the change means that people converting to v2 addons and plublishing won't force broken code on consumers with users on the affected systems decorators is ok in app land because there it's in the control of the team to adopt it if their support allows it, otherwise this can lead to v2 backlash and people staying away from the format which I find undesirable

looking for thoughts from others on this, thank you

NullVoxPopuli commented 6 months ago

I disagree, as we are not shipping v2 addons directly to browsers.

It's the consuming app's responsibility to compile away modern available syntax. the consuming app should compile away static{} blocks if your targets include browsers which do not natively support the syntax.

if this isn't happening (which may be the case by your tried combination), it's likely a bug with ember-cli-babel!!! (which we should fix and get a patch out immediately!).

Do you have a reproduction?, maybe we can start with debugging there.

void-mAlex commented 6 months ago

It's the consuming app's responsibility to compile away modern available syntax. the consuming app should compile away static{} blocks if your targets include browsers which do not natively support the syntax.

there is no config that I found that allows you to do that in an app every combination is broken somehow, worst is that when it compiles it breaks at runtime

the problem is there is no way for the developer to know they need to do anything with ember-cli-babel even if there is a bug with it, which I doubt, but will try and put together a reproduction, which won't tell you much unless you still have access to an old safari, which is not easy feat

NullVoxPopuli commented 6 months ago

there is no config that I found that allows you to do that in an app

config/targets.js should be sufficient

won't tell you much unless you still have access to an old safari, which is not easy feat

We don't need safari, we'd ensure that targets that include old safari don't generate assets with the static block.

void-mAlex commented 6 months ago

We don't need safari, we'd ensure that targets that include old safari don't generate assets with the static block.

the transform from statick blocks through babel breaks at runtime

NullVoxPopuli commented 6 months ago

What's the error and stack?

void-mAlex commented 6 months ago

something like $$meta.descriptor is undefined when computed properties are being set up need to set up minimal repro and we can talk

NullVoxPopuli commented 6 months ago

I also have a browser stack account i can debug with, once the repro is up

void-mAlex commented 6 months ago

https://github.com/void-mAlex/ember-v2addon-test repro playground

NullVoxPopuli commented 6 months ago

confirmed fixed by upgrading to a compatible version of ember-cli-babel 8.2.

https://github.com/emberjs/ember-cli-babel/releases/tag/v8.2.0