Meteor-files: Docs say to extend default schema, but default schema is not availble till after creation

[coagmano]

I have a suggestion:

Describe your feature / request

Export the default schema so it can be extended before it is passed to the constructor.

How you going to use it? Give a usage example(s)

To simplify the process of extending the default schema in a collection like so:

import { FilesCollection, defaultSchema } from 'meteor/ostrio:files';
import SimpleSchema from 'simpl-schema';

export const Images = new FilesCollection({
  collectionName: 'Images',
  schema: new SimpleSchema ({
    ...defaultSchema,
    tags: Array,
    createdBy: {
        type: String,
        denyUpdate: true,
        denyInsert: true,
        optional: true,
        autoValue() {
            if (this.isInsert) {
                return Meteor.userId();
            } else if (this.isUpsert) {
                return { $setOnInsert: Meteor.userId() };
            } else {
                this.unset();
            }
        },
    },
  }),
});
Images.collection.attachSchema(new SimpleSchema(Images.schema));

Happy to write a PR for this if you're happy for this to be added

[dr-dimitru]

Hello @coagmano ,

Great! Let's dig into it.
I suggest to make a schema as static property of FilesCollectionServer class. As we expect plain Object, perhaps it should be static getter. Actually I thought it's already made this way 😛

So we will be able:

import { FilesCollection } from 'meteor/ostrio:files';
import SimpleSchema from 'simpl-schema';

export const Images = new FilesCollection({
  collectionName: 'Images',
  schema: new SimpleSchema ({
    ...FilesCollection.schema,
    tags: Array,
    createdBy: {
        type: String,
        denyUpdate: true,
        denyInsert: true,
        optional: true,
        autoValue() {
            if (this.isInsert) {
                return Meteor.userId();
            } else if (this.isUpsert) {
                return { $setOnInsert: Meteor.userId() };
            } else {
                this.unset();
            }
        },
    },
  }),
});

Images.collection.attachSchema(new SimpleSchema(Images.schema));

WDYT?

[coagmano]

Yeah that's way cleaner than my suggestion, should be a super simple change too.
Any reason it shouldn't be on the Client as well?

EDIT: Looks like Client contains the default schema too

[dr-dimitru]

should be a super simple change too.

Agree.

Any reason it shouldn't be on the Client as well?

Security, no need to let everyone know what your schema looks like.
Do SimpleSchema requires "attaching schema" on client?

EDIT: Looks like Client contains the default schema too

You're right, but should not.

WDYT?

[coagmano]

I couldn't find any reference in simpl-schema or Collection2 about hiding the schema from the client, and I believe it requires it for client-side validation to run. The Meteor Guide also advises attaching the schema without specifying that you should hide it from the client.

I just checked and the other collections in my project also have their schema's exposed to the client.

I'd be nervous about removing the schema from the client as a result

Here's what the most basic change on both Server and Client looks like:
https://github.com/VeliovGroup/Meteor-Files/compare/dev...coagmano:add-static-schema-getter?expand=1

I haven't added any docs or changelog yet. Waiting till you're happy before I write it up

[dr-dimitru]

Looks, good. Have you tested it? Feel free to send a PR.

[coagmano]

Testing it in an app I'm building now. Everything seems okay. I'll let it run for a bit before I submit the PR to be sure

[dr-dimitru]

@coagmano merged and published as v1.9.3

[coagmano]

Eventually found an issue! There's a few keys that get added which were missing from the schema.
I've made a PR to add the missing keys: #542

[dr-dimitru]

Let's reopen it until an update is published.

[mksh-su]

Hello @coagmano ,

Thank you for PR, it was merged a while ago, perhaps we missed this exact issue.

[coagmano]

HI @mksh-su, the issue only remained open because a new release hasn't been cut yet.

AFAIK I'm the only one with the issue so there's no hurry to release, which is why this was pending

[dr-dimitru]

@coagmano whoops, my bad, I thought it was merged and published as v1.9.5, you're right https://github.com/VeliovGroup/Meteor-Files/commit/fe65debd2d1e191975d3b8e06efa06af7e727a6d is still waiting for merge. Going to publish new release in a day or two.

[coagmano]

Yeah I opened the PR the day after that release haha

[dr-dimitru]

Reopening to avoid missing it again :)

[dr-dimitru]

Closing, published as v1.9.6