Field and Accessor initializers should run after the field/accessor has been defined
Closed this issue · 12 comments
Currently, field/accessor initializers added via addInitializer run at the same time as method initializers. This means they run before their respective field/accessor is defined, which means if they intend to modify the field/accessor they cannot do so.
The ordering should be updated so that they run immediately after the field/accessor they were applied to is defined. Conceptually, this should not be a performance concern because it would be immediately after the init code for the field/accessor was run, so it should slot in at the same point in class instance creation, but we should double check that with implementers.
cc @syg
Gut reaction is this is probably fine? The "instance shape is statically analyzable" property still holds, it's just the timing of the initializer?
Yes, exactly, initializers for fields/accessors would just run immediately after the field/accessor is initialized and defined on the instance, no other changes
I thought the point of addInitializer was to run code before any initializers are run? Will changing this affect other use cases?
@rbuckton initialization prior to fields was for methods/accessors, which are defined on the prototype. Initialization of fields/accessors via addInitializer was actually not included in the proposal at first because it was seen as redundant, but we decided to include it to simplify the API. There is now a use case for initializers added to fields/accessors, see #508 (comment)
Sorry, I have been out of the loop for a while, so correct me if wrong, but I think in MobX we did run into the same limitation: https://github.com/mobxjs/mobx/pull/3638/files#diff-6df715307a8821749b3d5d5b0e8a6ad7360ed2fae4b01f7005b6c7b3efb3336bR69-R108
Edit: can't make it to the meeting, but sounds like I'm in favour of the change :)
This change may help us with accessor initialization as well.
We have a case where on construction we want to read and delete instance properties that are shadowing accessors and write the value back though the accessor. (this may seem odd, but the instance exists before the constructor is called, and in this case someone has assigned properties to what will be auto-accessors after construction).
We would like to just add an initializer that does this operation, but we can't write to the accessor in an initializer because the private storage for the accessor hasn't been installed yet, resulting in an error. So we have to write the value asynchronously, which is a bit of a convoluted code path and leaves the object in a weird state for a tick.
So just to confirm that this helps us, an initializer for an accessor added with addInitializer() (what about init()?) will run after the private storage is installed so it can access the accessor, yes?
So just to confirm that this helps us, an initializer for an accessor added with
addInitializer()will run after the private storage is installed so it can access the accessor, yes?
Yes, correct
what about
init()?
init will run during initialization of the accessor, before it is defined, just like it does currently
Was this resolved at yesterday's meeting?
Nobody else was available so we didn’t discuss it. My plan is to bring the change to committee at the next plenary I’m able to attend.
@justinfagnani how would being able to access the auto-accessor storage help with custom element pre-upgrade values? Can you describe that in more detail?
I have a different case:
There I got runtime errors because my decorator wants to know initial values of getters, but fails when getters read a private field (and will silently fail with public instead of private fields due to fields not being initialized yet). The example there would work nicely if the addInitializer initializer ran after fields.
Judging from conversations, it seems like there are uses cases for running addInitializer initializers before or after fields (f.e. what if a field wants to use a method, and the method decorator should initialize it so that it is ready before fields are initialized?), but it seems most people want them after.
Would it make more sense to change addInitializer in the current spec to be after fields? Or more sense to add an alternative API that lets people choose before/after fields as described in #521?