ef4 / decorator-transforms

Better babel transforms for decorators
8 stars 5 forks source link

Idea for wider browser support #3

Closed ef4 closed 5 months ago

ef4 commented 10 months ago

Suggestion via @nickschot: to support browsers without static blocks, we could use static private field initializers to get the same timing.

mansona commented 10 months ago

I've just recently been reminded about the new policy for browser support. Sure this new transform is an opt-in thing but we should probably try to target as old a safari as possible with this and then open an RFC to make that the oldest supported safari we support so that Ember 6.0 can drop support before that.

Essentially right now we technically support Safari 12 because it needs to be manually updated https://rfcs.emberjs.com/id/0685-new-browser-support-policy https://emberjs.com/browser-support/

lifeart commented 8 months ago

upd: Oh, sorry, misread title, this is how I able to get working this plugin in browser itself:

I explored this area, this is compatible code: https://github.com/lifeart/glimmer-next/pull/25/files#diff-3797daadde243706ac174b12708347f5aeffddd6d1ae20e57d3e877655a9b487

TLDR: have to remove

import { createRequire } from "node:module";
import { ImportUtil } from "babel-import-util";
import { globalId } from "./global-id.ts";
const req = createRequire(import.meta.url);
const { default: decoratorSyntax } = req("@babel/plugin-syntax-decorators");

and replace to

import type { types as t, NodePath } from '@babel/core';
import { ImportUtil } from 'babel-import-util';
import { globalId } from "./global-id.ts";
import * as decorators from '@babel/plugin-syntax-decorators';
const decoratorSyntax = decorators.default.default;
davidtaylorhq commented 8 months ago

I took a stab at implementing this in #17 - it works.

However, along the way I realised that it might not be necessary. When an older browser version (e.g. safari 15) is specified in config/targets.js in an ember app, @babel/preset-env will automatically apply babel-plugin-transform-class-static-block.

That transform does exactly what was proposed in this issue, and doesn't seem to have any dependency on other babel transforms:

A class with a static block will be transformed into a static private property, whose initializer is the static block wrapped in an IIAFE (immediate invoked arrow function expression).

Relying on this existing babel transform makes a lot of sense to me - it allows each app developer to choose control their own browser support.

An approach like #17 would only be needed if decorator-transforms needs to work totally by itself across a wide range of browsers.

ef4 commented 8 months ago

@babel/preset-env will automatically apply babel-plugin-transform-class-static-block.

When I checked last this didn't work in a single pass. It does work in, for example, a v2 addon that is using decorator-transforms in its prepublication build, so that the app's later pass can do the preset-env.

But we should check again and confirm, in the various permutations (classic vs embroider, app vs addon, targets that support private fields but not static blocks vs targets that support neither).

davidtaylorhq commented 8 months ago

When I checked last this didn't work in a single pass

Interesting! I confirmed the same in a vanilla Embroider app.

It does work on Discourse's app code, which suggests we have some kind of double-babel loop going on for our app code. 😬

Looking at the vanilla app, it seems that transform-class-static-block does run after decorator-transforms. However, that implementation is hooked onto ClassBody rather than being hooked onto StaticBlock. I guess that means it won't transform StaticBlocks introduced by prior Babel plugins.

So I guess the decision here is: do we try to make it work via transform-class-static-block somehow? Possibly with some upstream changes in Babel?

Or do we go with the 'easy' option of landing #17, which uses private class fields unconditionally.