contentful-migrate icon indicating copy to clipboard operation
contentful-migrate copied to clipboard

Provide --compiler flag for `up`, `down`, `list`

Open trptcolin opened this issue 5 years ago • 6 comments

This allows folks to write migrations in TypeScript, with a setup like the following:

  • A file migrations/tsnode.js containing a require-able module:
require("ts-node/register")
module.exports = () => {}

See https://github.com/tj/node-migrate/issues/108#issuecomment-371107685 for more here.

  • A command-line argument to register the ts extension with ts-node:
$ ctf-migrate list --compiler "ts:./migrations/tsnode.js" \
    --space-id $CONTENTFUL_SPACE_ID \
    --environment-id $CONTENTFUL_ENVIRONMENT_ID \
    --access-token $CONTENTFUL_MANAGEMENT_ACCESS_TOKEN 
    -a

And just to reiterate some of the tradeoffs I see from the related issue (#53):

Pros

  • Gets TypeScript migrations working.
  • Fairly uninvasive.

Cons

  • Requires users to add a file, and point to it from the command-line argument. We could alternatively make the contentful-migrate flag be from more of a closed set to make it user-friendlier (e.g. something like --compiler typescript), but I'm more inclined to leave it open to extension in letting more options besides typescript potentially work.
  • May mean breaking changes if/when tj/node-migrate changes their API, although they're already thinking/talking about handling breaking changes with major version bumps.
  • Opens up questions about whether the migration codegen should support TypeScript.

No worries if you want to go a different way - just figured showing the code might be easier than talking about it (also might save others some time).

trptcolin avatar Feb 09 '20 13:02 trptcolin

One additional caveat I just ran into that I wanted to call out:

  • ctf-migrate bootstrap writes *.js files which are easy enough to rename, but because it also stores the complete filename in Contentful, users need to edit that migration content directly (the symptom is errors like error : Error: Missing migration file: 20200210202532-create-blog-post.js when attempting to migrate).

trptcolin avatar Feb 10 '20 22:02 trptcolin

Hey, thanks very much for this, and sorry for not replying earlier.

Even though I like the simple/open approach you took, I think I'd like to have something like a flag in bootstrap (something like --language typescript), and have the up/down/list commands automatically detect the language based on the extension (.ts, .js, .coffee...). That way we could detect which language was used and use the appropriate compiler, without the need to specify an additional argument in every command.

We could have this approach and also keep the --compiler flag you proposed for open extensibility, as long as we can solve the Error: Missing migration file: 20200210202532-create-blog-post.js issue.

Thoughts?

deluan avatar Mar 10 '20 20:03 deluan

@deluan Yep, absolutely, makes sense. It'll definitely be user-friendlier and seems like that approach could also allow migrations to be run in different languages (e.g. project started off in javascript but migrated to typescript a couple years later, and they don't have to rewrite old migrations to get them running).

Just to set expectations & make sure nobody's waiting on me: I'm probably not going to tackle this anytime soon, just depending on my fork for now, which is going fine for my purposes. Feel free to close or repurpose this PR!

trptcolin avatar Mar 10 '20 21:03 trptcolin

Hello, will we get typescript support? for new projects that file error might not be an issue.

fr0609 avatar Sep 07 '20 23:09 fr0609

I'll take a stab at this, hopefully will have a new release by end of this week

deluan avatar Sep 09 '20 15:09 deluan

Hey @fr0609 and @trptcolin, sorry for the delay. I'm not a TypeScript user myself, so testing this (setting up the dev env, etc..) is not something I've been able to do in my free time.

What do you guys think about @devlato's approach on PR https://github.com/deluan/contentful-migrate/pull/131?

deluan avatar Oct 17 '20 14:10 deluan