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

React Native: create tables query fails in release mode with query syntax error #3933

Closed
naoey opened this issue Apr 3, 2019 · 13 comments
Closed
Labels

Comments

@naoey
Copy link

naoey commented Apr 3, 2019

Issue type:

[ ] question
[x] bug report
[ ] feature request
[ ] documentation issue

Database system/driver:

[ ] cordova
[ ] mongodb
[ ] mssql
[ ] mysql / mariadb
[ ] oracle
[ ] postgres
[ ] cockroachdb
[ ] sqlite
[ ] sqljs
[x] react-native
[ ] expo

TypeORM version:

[ ] latest
[ ] @next
[x] 0.2.16 (or put your version here)

Steps to reproduce or a small repository showing the problem:

I only added TypeORM to my project last week. Everything was working great while developing but as soon as I began making release builds of my app the entire database stopped working. Turning on logging shows me this:

2019-04-03 19:08:05.864 [error][tid:com.facebook.react.JavaScript] 'query failed: ', 'CREATE TABLE "temporary_t" ()'
2019-04-03 19:08:05.866 [error][tid:com.facebook.react.JavaScript] 'error: ', { message: 'near ")": syntax error', code: 5 }

I've been unable to identify what could be causing this and from a cursory search I haven't been able to find any other reports of this issue.

I believe the error is being thrown immediately on creation of the database in the root of the app, and no further database related operation work in the application. My initialisation code:

const sqliteConnectionManager = getConnectionManager();
const connection = sqliteConnectionManager.create({
  type: 'react-native',
  database: DATABASE_NAME,
  synchronize: true,
  logging: true,
  // migrations,
  entities,
  location: 'default',
});

await connection.connect();

I'm using plain JavaScript in this project, not TypeScript. Any help would be much appreciated.

May also be worth mentioning I'm using ES6 with decorators for defining my entities, like so:

@Entity()
class ReferenceDataGroup {
  @PrimaryColumn('text')
  groupId;

  @Column({ type: 'text', nullable: true })
  lastModified;

  @OneToMany(() => ReferenceDataValue, rdValue => rdValue.groupId, { eager: false })
  values;
}
@naoey
Copy link
Author

naoey commented Apr 4, 2019

I tried changing all my entities to use EntitySchema as given in your JavaScript examples. The syntax error logs have now stopped but the database connection is still not being maintained in release mode. It closes immediately after being opened:

2019-04-04 07:54:08.738 [info][tid:com.facebook.react.JavaScript] OPEN database: <db_name>
2019-04-04 07:54:08.740 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:197] target database location: nosync
2019-04-04 07:54:08.740 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:208] Opening db in mode READ_WRITE, full path: ~/Library/Developer/CoreSimulator/Devices/E332D2E7-DD92-4F5B-8682-DDE1CB435F95/data/Containers/Data/Application/6676A6AB-6CC9-4074-9F3C-72E2F59CAD4D/Library/LocalDatabase/<db_name>
2019-04-04 07:54:08.741 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:235] Database opened
2019-04-04 07:54:08.741 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:249] Good news: SQLite is thread safe!
2019-04-04 07:54:08.744 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:255] open cb finished ok
2019-04-04 07:54:08.745 [info][tid:com.facebook.react.JavaScript] CLOSE database: <db_name>
2019-04-04 07:54:08.746 [info][tid:com.facebook.react.JavaScript] closing db with transaction queue length: 0
2019-04-04 07:54:08.746 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:294] close: database still exists at path ~/Library/Developer/CoreSimulator/Devices/E332D2E7-DD92-4F5B-8682-DDE1CB435F95/data/Containers/Data/Application/6676A6AB-6CC9-4074-9F3C-72E2F59CAD4D/Library/LocalDatabase/<db_name> proceeding to close it.
2019-04-04 07:54:08.746 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:302] close: closing db: <db_name>
2019-04-04 07:54:08.746 [info][tid:com.facebook.react.SQLiteQueue][SQLite.m:309] database file still exists after close

Once again, it all works absolutely fine in debug mode.

@Kononnable
Copy link
Contributor

If it's working and debug and not in prod then it has something to do with configs. I would suggest disabling minification and mangling and see if that helps. It's most common problem in such cases.
I'm not using react-native, so don't have much knowledge on how are configured debug and prod modes there.

@naoey
Copy link
Author

naoey commented Apr 20, 2019

The table creation issue was indeed caused by minification it seems. I was able to continue using the decorator pattern by explicitly mentioning names for all entities and columns like @Column({ name: 'groupId', type: 'text' }).

The issue in my second comment persisted though. I wasn't able to get to the bottom of that one.

@Kononnable
Copy link
Contributor

I didn't found any related issues. I'm not very familiar with react native, so I probably won't be able to help much more.
I can only suggest to 'merge' configs - try moving(merging) config from dev to prod and see after changing which config setting it started to work again.

@naoey
Copy link
Author

naoey commented Apr 23, 2019

Considering my original problem was resolved I suppose this issue can be closed.

Though it may be worth adding a quick but prominent mention in the setup for React Native that when using the decorator pattern for defining entities with ES6 and Babel, explicit table and column names need to be defined if the compiled code is going to be minified. Since minification is on by default in RN projects, I imagine other users may keep running into this issue.

@tindn
Copy link

tindn commented Apr 24, 2019

@naoey Do you have migrations specified in your connection config?

I was having the same issue, and it turns out in Release build, the js is minified, and the class names for migrations are mangle. However, these need to have the timestamp as specified here
https://github.com/typeorm/typeorm/blob/master/src/migration/MigrationExecutor.ts#L284

So I found the solution here
facebook/metro#154 (comment)

Simply put, I just installed a different minifier from the default one

yarn add -dev metro-minify-terser

and then added this in the transformer section of the metro config file

minifierPath: 'metro-minify-terser',
minifierConfig: {
      keep_fnames: true,
},

@naoey
Copy link
Author

naoey commented Apr 25, 2019

No I don't have any migrations in my connection right now. However class and class property names being mangled was the reason for this issue regardless, since that essentially destroyed all my entity and column names. So your solution might also be an viable alternative.

@naoey
Copy link
Author

naoey commented May 9, 2019

I don’t believe it is possible for TypeORM to fix this. At the point this code actually runs the names are already mangled by minification. Only way I see is for the user of the library to explicitly mention column and table names as I mentioned in my previous comment.

@peter-axomic
Copy link

This was a problem for us too. We are using Expo.io which doesn't allow you to change how things are minified when deployed, and therefore all migrations break in production.

We forked 0.2.16 and hacked in a fix (axomic@febb50c), but would love to get a fix into the main repo.

The problem is in:

src/migration/MigrationExecutor.ts

getMigrations() uses introspection on the constructor to get the class name, but if the code is minified the class name is typically just a single letter!

const migrationClassName = (migration.constructor as any).name;

We added an optional "name" property to MigrationInterface so that we could explicitly name a migration, rather than introspecting the class name, and then changed the line above to:

const migrationClassName = migration.name || (migration.constructor as any).name;

We aren't the only ones with this issue, here are a few examples of people having to find workarounds.

facebook/metro#154 (comment)
#2549 (comment)
typeorm/ionic-example#19

@Kononnable
Copy link
Contributor

I think this fix isn't a part of typeorm package because nobody made a PR for it.
I new name field should be optional(to not break already generated migrations). We should also think if any changes need to be done when cli generate new migrations(is name option gonna be the default one now).

As for original issue - minification takes place before TypeOrm code is run, so unfortunately we can do nothing about that.

leonardofalk added a commit to wonder-oss/typeorm that referenced this issue Oct 10, 2019
@crutchcorn
Copy link
Contributor

I think the documentation here needs to be improved. Sure enough, I ran into this same issue only on the built version and updating each Entities and Columns fixed the problem, but only after an hour or so of searching did I find this GH issue

@allandiego
Copy link
Contributor

The error occurs because of minify/uglify process, workaround was disable this for class and function names:

node_modules\metro-config\src\defaults\index.js

minifierConfig: {
      keep_classnames: true,  // enable to fix typeorm
      keep_fnames: true,  // enable to fix typeorm
      mangle: {
        // toplevel: false,
        keep_classnames: true, // enable to fix typeorm
        keep_fnames: true, // enable to fix typeorm
      },
      output: {
        ascii_only: true,
        quote_style: 3,
        wrap_iife: true
      },
      sourceMap: {
        includeSources: false
      },
      toplevel: false,
      compress: {
        // reduce_funcs inlines single-use functions, which cause perf regressions.
        reduce_funcs: false
      }
    },

still looking for a better solution since the mentioned package metro-minify-terser is not avaliable anymore, im thinking in change this configurations via metro.config, or switch minify package for https://github.com/terser/terser

@kristfal
Copy link

Just a heads up here: I ran into a related issue, but it was caused by the queryBuilder. While minification seems to work for regular queries as the minified aliases would make queries consistent, using the queryBuilder with hardcoded strings would break due to the following:

The builder would be:

getConnection()
    .getRepository(Table)
    .createQueryBuilder()
    .where('table.id = :id', {
      id,
    })
    .getMany();

A generated select query would be

SELECT t.id from table t WHERE table.id = 1

which would fail for obvious reasons. The only proper solution is to update minifireConfig via metro.config.js, ex:

module.exports = {
  transformer: {
    minifierConfig: {
      keep_classnames: true, // enable to fix typeorm
      keep_fnames: true, // enable to fix typeorm
      mangle: {
        toplevel: false,
        keep_classnames: true, // enable to fix typeorm
        keep_fnames: true, // enable to fix typeorm
      },
      output: {
        ascii_only: true,
        quote_style: 3,
        wrap_iife: true,
      },
      sourceMap: {
        includeSources: false,
      },
      toplevel: false,
      compress: {
        reduce_funcs: false,
      },
    },
  },
};

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

No branches or pull requests

7 participants