Support full-text index decorator in babel plugin #6218
Conversation
- Check for `index` decorator calls that are passed the string "full-text" and subsequently add the `{ indexed: "full-text" }` property to the schema.
- Update README with description and example of indexing a string field by full text
- Update tests to verify schemas with full-text indexing are properly generated
|
@atdyer Thank you for your contribution. It looks good to me, and I have requested the team to review your PR. I like that you have taken the time to update Some test suites will fail since you don't have permission to launch some component - don't worry about that. Once merged, we will update |
| const indexCall = | ||
| indexDecoratorCall | ||
| && types.isStringLiteral(indexDecoratorCall.callExpression.arguments[0]) | ||
| && indexDecoratorCall.callExpression.arguments[0].value === 'full-text' |
There was a problem hiding this comment.
I'm contemplating if we actually want to pass through any expression here and handle validation at runtime instead 🤔
There was a problem hiding this comment.
It should just forward whatever is given to the schema. Then at least we get a run-time error.
Unfortunately it's not possible to get type errors here, which would be great.
| it('ignores `@index()` decorators with invalid parameters', () => { | ||
| const transformCode = transform({ | ||
| source: `import Realm, { Types, BSON, List, Set, Dictionary, Mixed } from "realm"; | ||
| export class Person extends Realm.Object { @Realm.index("fulltext") name: Realm.Types.String; }`, | ||
| }); | ||
| const parsedSchema = extractSchema(transformCode); | ||
|
|
||
| expect((parsedSchema?.properties.name as PropertySchema).indexed).toBeUndefined(); | ||
| }) |
There was a problem hiding this comment.
To make the code more robust to future changes and allow for the SDK to report this as an error (instead of silently not recognising the users intent) we might want this passed into the schema.
What's your take on this @takameyer?
There was a problem hiding this comment.
I agree that an error should be thrown in this case.
There was a problem hiding this comment.
Helping the user understand what's wrong and how to correct it also aligns better with the goals of the updated schema parser 👍
There was a problem hiding this comment.
I would approve this after the comments from @kraenhansen are resolved.
| it('ignores `@index()` decorators with invalid parameters', () => { | ||
| const transformCode = transform({ | ||
| source: `import Realm, { Types, BSON, List, Set, Dictionary, Mixed } from "realm"; | ||
| export class Person extends Realm.Object { @Realm.index("fulltext") name: Realm.Types.String; }`, | ||
| }); | ||
| const parsedSchema = extractSchema(transformCode); | ||
|
|
||
| expect((parsedSchema?.properties.name as PropertySchema).indexed).toBeUndefined(); | ||
| }) |
There was a problem hiding this comment.
I agree that an error should be thrown in this case.
| const indexCall = | ||
| indexDecoratorCall | ||
| && types.isStringLiteral(indexDecoratorCall.callExpression.arguments[0]) | ||
| && indexDecoratorCall.callExpression.arguments[0].value === 'full-text' |
There was a problem hiding this comment.
It should just forward whatever is given to the schema. Then at least we get a run-time error.
Unfortunately it's not possible to get type errors here, which would be great.
| it('ignores `@index()` decorators with invalid parameters', () => { | ||
| const transformCode = transform({ | ||
| source: `import Realm, { Types, BSON, List, Set, Dictionary, Mixed } from "realm"; | ||
| export class Person extends Realm.Object { @Realm.index("fulltext") name: Realm.Types.String; }`, | ||
| }); | ||
| const parsedSchema = extractSchema(transformCode); | ||
|
|
||
| expect((parsedSchema?.properties.name as PropertySchema).indexed).toBeUndefined(); | ||
| }) |
There was a problem hiding this comment.
Helping the user understand what's wrong and how to correct it also aligns better with the goals of the updated schema parser 👍
Removes check that the specific string "full-text" is the only allowable parameter to the `@index` decorator. New behavior is to allow any string to be passed through to the schema. Invalid values in the schema will throw at runtime, alerting user to possible errors.
|
@takameyer I believe I've addressed all your comments - now any string passed into the |
There was a problem hiding this comment.
As far as I can tell, this just needs an entry in the changelog.
This closes #5800 by adding support for the
index("full-text")decorator.indexdecorator calls that are passed the string "full-text" and subsequently add the{ indexed: "full-text" }property to the schema.