Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional name to MigrationInterface (fixes #3933) #4873

Merged
merged 1 commit into from
Oct 18, 2019
Merged

Add optional name to MigrationInterface (fixes #3933) #4873

merged 1 commit into from
Oct 18, 2019

Conversation

leonardofalk
Copy link
Contributor

@leonardofalk leonardofalk commented Oct 8, 2019

This fixes a problem when after minification process, migrations cannot be loaded because their name is changed to some random value, this happens mostly when using react-native/expo.

There are several topics about this issue in facebook/metro and typeorm repositories, but I couldn't find any pull request for it. It could be fixed by changing the default minifier, but that can break other stuff.

There are also a few steps before merging this PR:

  • If this field sould be called 'migrationName'? Other options could be 'displayName' or simply 'name'.

  • Should migrations generated with the CLI contain this attribute by default?

EDIT

Created a babel-plugin to automatically add name property:

module.exports = function({ types: t }, options) {
  return {
    visitor: {
      ClassDeclaration(path) {
        const migrationName = t.stringLiteral(path.node.id.name)
        const migrationTimestamp = parseInt(migrationName.value.substr(-13), 10)

        if (!isNaN(migrationTimestamp)) {
          const prop = t.classProperty(t.identifier(options.property), migrationName, null, null)

          path.node.body.body.push(prop)
        }
      },
    },
  }
}
{
  presets: ['module:metro-react-native-babel-preset'],
  plugins: [
    ['babel-plugin-typeorm-migration-name', { property: 'migrationName' }]
  ],
}

@pleerock
Copy link
Member

If this field sould be called 'migrationName'? Other options could be 'displayName' or simply 'name'.

simply name looks fine

Should migrations generated with the CLI contain this attribute by default?

not sure, but we can add it

@leonardofalk
Copy link
Contributor Author

If this field sould be called 'migrationName'? Other options could be 'displayName' or simply 'name'.

simply name looks fine

Should migrations generated with the CLI contain this attribute by default?

not sure, but we can add it

Done, changed to name and added it to the migration template.

@leonardofalk leonardofalk changed the title Add optional migrationName to MigrationInterface (fixes #3933) Add optional name to MigrationInterface (fixes #3933) Oct 14, 2019
@pleerock pleerock merged commit 4a73fde into typeorm:master Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants