stampit-org / stamp

Stamps - better OOP model
https://stampit.js.org
MIT License
25 stars 3 forks source link

Init-property vs. injecting stamps as properties #46

Open MaximusHaximus opened 6 years ago

MaximusHaximus commented 6 years ago

I've made a few stamps where I inject other stamps as properties using .props({...}) for dependency injection purposes (testing/mocking).

I'd like to use init-properties stamp to automatically initialize some props that are stamps and have their arguments automatically namespaced (👍 ), but in some cases in my code, I am injecting stamps as .props() where those stamps are not intended to be initialized during initialization of the parent stamp. e.g.

const MyStamp = require('./MyStamp');

const asyncStampFactory = stampit
.init(function(fetchRemoteStampConfiguration, numStamps) {
 this.fetchRemoteStampConfiguration = fetchRemoteStampConfiguration;
 this.numStamps = numStamps;
})
.props({ myStamp })
.methods({
  makeSomestamps() {
    const stamps = [];

    for(var x = 0; x < numStamps x += 1) {
      const stampConfig = await this.fetchRemoteStampConfiguration(x)
      stamps[x] = this.myStamp(stampConfig);
    }
    return  stamps;
  }
});

return asyncStampFactory;

Note that I won't be making a single stamp from the myStamp prop, but an array of them -- and I can't create the array of stamps on stamp initialization as the point of this stamp is to do some async fetching and return instances of the myStamp per result that was fetched.

init-properties seems to conflict with this.

I'm wondering if there might be a justification here for being able to configure the init-property stamp not to automatically initialize ALL stamp properties, but only some?

My instinct was to move myStamp to .configuration() and then use this.config.myStamp() in the loop to create instances instead, and only put stamps in .props() that should be auto initialized with values that are available during stamp initialization.

Would appreciate any insights :)

koresar commented 6 years ago

Yeah, the init-properties initialization logic is too hungry. I'd agree that it's better to configure it.

How about the following API:

import InitProperty, { initProperty } from '@stamp/init-property';

const StampA = initProperty({ foo: StampB } );
// or
const StampA = InitProperty.initProperty({ foo: StampB } );

What do you think?

koresar commented 6 years ago

Tip 1. This array const stamps = []; is not an array of stamps. These are object instances. You are creating instances by calling a stamp myStamp(stampConfig);.


Tip 2. Stamp initializer have a bit different signature. https://stampit.js.org/api/initializers#initializer-arguments

As you can see all you need is to add curlies:

.init(function( { fetchRemoteStampConfiguration, numStamps } ) {

Tip 3. You can make your initializer async without adding makeSomestamps method.

const MyStamp = require('./MyStamp');

const asyncStampFactory = stampit
.init(async function({ fetchRemoteStampConfiguration, numStamps }) {
  const instances = [];
  for(var x = 0; x < numStamps x += 1) {
    const stampConfig = await fetchRemoteStampConfiguration(x);
    instances[x] = this.myStamp(stampConfig);
  }
  return instances;
})
.props({ myStamp });

const instances = await asyncStampFactory({ fetchRemoteStampConfiguration, numStamps: 10 });

There is one downside - this initializer must be the last initializer in the composition sequence (add no more initializers after it).

MaximusHaximus commented 6 years ago

Hey, thanks for the tips :) My example was hastily written, so my apologies for the inconsistency in wording (instances vs. stamps) and the inaccurate init() function signature. It was merely to provide the scaffolding for my use case of needing to inject a stamp using .props(), but exclude it from being automatically initialized by init-property :) . Your suggested init-property changes look like exactly what I'd envisioned as a perfect solution.