ViacomInc / data-point

JavaScript Utility for collecting, processing and transforming data
Apache License 2.0
67 stars 34 forks source link

Do not resolve transform entities with BaseEntity#resolveEntity #175

Open raingerber opened 6 years ago

raingerber commented 6 years ago

Problem description:

Transform entities have a value property, but they don't have lifecycle methods like inputType, before, after, and outputType:

https://github.com/ViacomInc/data-point/blob/master/packages/data-point/lib/entity-types/entity-transform/factory.js#L17

Despite this, transforms are resolved with the same function as other entities, which means they pass through a long promise chain where most of the promises do nothing:

https://github.com/ViacomInc/data-point/blob/master/packages/data-point/lib/entity-types/base-entity/resolve.js#L141

Possible solutions:

  1. Do not resolve transform entities with resolveEntity (which would also mean you can't use middleware for transforms).

  2. Refactor resolveEntity so it does not create promises for nonexistent lifecycle methods (I haven't thought of a great way to do this).

  3. Deprecate transform entities (or encourage not to use them). They're basically a way to name reducers, but doing this works too:

const reducer = [
  '$a',
  (value) => value * 2
]

dataPoint.transform(reducer, { a: 1 })

Since this also creates a JS variable, it's possible to use editor tools like jumping from a variable reference to the variable declaration, which is not possible when declaring it like this:

DataPoint.create({
  entities: {
    'transform:the-same-reducer': [
      '$a',
      (value) => value * 2
    ]
  }
})
acatl commented 6 years ago

i have a solution i want to implement in baseentity resolve that would solve this problem

paulmolluzzo commented 6 years ago

This is one of only two issues left for the v2.x milestone, but it's still a draft. Have we come up with a solution we want to implement?

i have a solution i want to implement in baseentity resolve that would solve this problem

@acatl are you planning to do this one? Maybe you can be assigned?