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

Typescript migrations #53

Open
shennyg opened this issue Nov 4, 2019 · 10 comments
Open

Typescript migrations #53

shennyg opened this issue Nov 4, 2019 · 10 comments
Labels
enhancement New feature or request

Comments

@shennyg
Copy link

shennyg commented Nov 4, 2019

It looks like tj/node-migrate#144 needs to be merged in before we can get typed migrations working.

@deluan
Copy link
Owner

deluan commented Nov 8, 2019

I'll have to spend some time experimenting with this. Reading the above PR, and its associated issues, seems that there are alternative ways for implementing TS migrations

@deluan deluan added the enhancement New feature or request label Nov 8, 2019
@trptcolin
Copy link

@deluan Any interest in / openness to a PR that provides a workaround for using TypeScript?

Pros:

  • Gets TypeScript migrations working.
  • Fairly uninvasive: adds a flag to each relevant command (AFAICT: just list, up, down) and delegates to the existing registerCompiler stuff in tj/node-migrate) for the actual work.

Cons:

  • Requires users to add a file like the one here, 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 as described in the above ticket, 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.

Thoughts? I'd love to be using TypeScript, but this tool seems like a must for Contentful migrations. Nice work!

@devlato
Copy link

devlato commented Jun 26, 2020

Hey folks – any updates on this? I'm very interested in getting TS migrations supported 😄 😅

@devlato
Copy link

devlato commented Jun 26, 2020

In meanwhile, I forked this repo to add support for custom templates (with --migration-template option/-t option/TEMPLATE_FILE env variable) and custom generated file extensions (with --extension option/-e option/MIGRATION_FILE_EXTENSION env variable) and published a library fork named https://www.npmjs.com/package/contentful-migrate-fork. Additionally, I've created a PR for merging the changes into the original library.

@nicam
Copy link

nicam commented Oct 30, 2020

+1 for Typescript support :)

@colinwirt
Copy link

Is there a level of simplified PR that you're likely to have time to review @deluan to make this package more TypeScript-friendly?

I've found it fine to use this package with ts-node, so might start with simply supporting ts template generation.

@deluan
Copy link
Owner

deluan commented Mar 5, 2021

I have to admit I haven't had much time recently to work on this project, as I'm not actively using Contentful anymore. Also it does not help that I don't have much experience with TS, and not much opportunities to experiment with it.

I put my comments on @trptcolin 's PR with my thoughts on the approach I'd like to implement, and I think (IMHO), that it should be straightforward to do that way

What does not help is that seems like the integration tests are broken, and I have to spend some time fixing them before anything else is done in the project.

@colinwirt
Copy link

Thanks @deluan :)

What does not help is that seems like the integration tests are broken, and I have to spend some time fixing them before anything else is done in the project.

I was thinking that that might be something holding up approvals. Is that something I can help with fixing?

@deluan
Copy link
Owner

deluan commented Mar 8, 2021

Yes it is, and help would be very welcome, thanks for offering it! Send me an email if you decide to tackle it and need some help on setting up the environment to run integration tests locally.

@bgschiller
Copy link

bgschiller commented Mar 28, 2022

We can get acceptable typescript support with only a couple of changes:

  1. Add // @ts-check to the top of the migration template
  2. Add the following comment block above both exports.up and exports.down:
/**
 * @param {import("contentful-migration").default} migration
 */

This is reasonable non-invasive, and still allows us to use Typescript checking without waiting on contentful to land changes to contentful-migrations. I'm happy to prepare a PR for this change if you like.

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

No branches or pull requests

7 participants