Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Init-property vs. injecting stamps as properties #46

Open
MaximusHaximus opened this issue May 25, 2018 · 3 comments
Open

Init-property vs. injecting stamps as properties #46

MaximusHaximus opened this issue May 25, 2018 · 3 comments

Comments

@MaximusHaximus
Copy link

MaximusHaximus commented May 25, 2018

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
Copy link
Member

koresar commented May 26, 2018

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
Copy link
Member

koresar commented May 26, 2018

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants