Skip to content
This repository has been archived by the owner on Jun 16, 2023. It is now read-only.

Provide --compiler flag for up, down, list #82

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

trptcolin
Copy link

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 tj/node-migrate#108 (comment) 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).

This allows TypeScript migrations, with a setup like the following:

- A file `migrations/tsnode.js` containing a require-able module:

    require("ts-node/register")
    module.exports = () => {}

  See
  tj/node-migrate#108 (comment)
  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
@trptcolin
Copy link
Author

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).

@deluan
Copy link
Owner

deluan commented Mar 10, 2020

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?

@trptcolin
Copy link
Author

@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!

@fr0609
Copy link

fr0609 commented Sep 7, 2020

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

@deluan
Copy link
Owner

deluan commented Sep 9, 2020

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

@deluan
Copy link
Owner

deluan commented Oct 17, 2020

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 #131?

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

Successfully merging this pull request may close these issues.

3 participants