deluan / contentful-migrate

🐎 Schema migration tooling for Contentful, with state management
https://www.contentful.com/blog/2018/09/13/content-model-changes-scale-telus-cms-as-code/
MIT License
60 stars 37 forks source link

Typescript migrations #53

Open shennyg opened 4 years ago

shennyg commented 4 years ago

It looks like https://github.com/tj/node-migrate/pull/144 needs to be merged in before we can get typed migrations working.

deluan commented 4 years ago

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

trptcolin commented 4 years ago

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

Pros:

Cons:

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

devlato commented 4 years ago

Hey folks – any updates on this? I'm very interested in getting TS migrations supported πŸ˜„ πŸ˜…

devlato commented 4 years ago

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 commented 3 years ago

+1 for Typescript support :)

colinwirt commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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.