pbomb / flow-immutable-models

Generates model classes from Flow types using Immutable.js
42 stars 8 forks source link

Add comment once per generated code block, automatically add (some) necessary imports #17

Open object88 opened 7 years ago

object88 commented 7 years ago

Should address #4

object88 commented 7 years ago

@p-bomb suggested,

I think we should add the missing imports as the first non-commented line instead of after the cutline so we don't break common eslint rules like this one.

I'm not keen on breaking eslint rules, but it seems rude to modify the original code. Perhaps if we annotated the import with a comment indicating that it was generated?

pbomb commented 7 years ago

I don't think it's rude to modify the original code, because these model files exist in order to support the code generation. They go hand-in-hand. I don't think we should add commented-out import statements and I think if we add import statements later in the file, we might break eslint rules, which I think we need to avoid. import statements should go at the top of the file. If we're going to add support for that to this library, they should be added to the top of the file.

object88 commented 7 years ago

Sorry, I think I wasn't clear. I mean, do something like this...

Original:

export type QuuxModelType = {
  id: number,
  name: string,
};

Post code generation:

import * as Immutable from 'immutable'; // Added
import ImmutableModel from 'flow-immutable-models'; // Added

export type QuuxModelType = {
  id: number,
  name: string,
};

// //////////////////////////////////////////////////////////////////////////////
//
// NOTE: EVERYTHING BELOW THIS COMMENT IS GENERATED. DO NOT MAKE CHANGES HERE.
//
// If you need to update this class, update the corresponding flow type above
// and re-run the flow-immutable-models codemod
//
// //////////////////////////////////////////////////////////////////////////////
[blahblahblah]
object88 commented 7 years ago

(Also, I lied, this PR is very incomplete.) OK, more complete now.

pbomb commented 7 years ago

I'm fine with that, although I think the import comments are unnecessary.

object88 commented 7 years ago

OK; they may be tricky to pull off anyway. So I will concentrate on just getting the imports into the top of the file. I appreciate the feedback!