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

Incorrect Example Syntax - Dynamic References via refPath #11565

Merged
merged 1 commit into from
Mar 25, 2022

Conversation

chandiwalaaadhar
Copy link
Contributor

"on:" is a reserved keyword which fails on execution, therefore modelId: would be the correct key and works well

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request. The two fields below are mandatory.

If you're making a change to documentation, do not modify a .html file directly. Instead find the corresponding .pug file or test case in the test/docs directory.

Summary
This PR will make the documentation example codes bug free as the current syntax gave error Error: `on` may not be used as a schema pathname

Examples

NA

"on:" is a reserved keyword which fails on execution, therefore modelId: would be the correct key and works well
@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 24, 2022

Hmm. I would actually think that 'on' being a "reserved keyword" should be classified as a bug.

@AbdelrahmanHafez
Copy link
Collaborator

Reserved keywords should be removed from #10414, if you can't use on, that's something we should fix.

That being said, while we allow usage of keywords that were previously reserved, we recommend against it because it will break plugins that rely on using them, so the example should not mention a case where it might cause issues for the end-user.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 24, 2022

I think the problem is, that Schema/Model is an EventEmitter. So while you can set an attribute named "on", it collides with the on property of the EventEmitter.

@AbdelrahmanHafez AbdelrahmanHafez added the docs This issue is due to a mistake or omission in the mongoosejs.com documentation label Mar 25, 2022
@AbdelrahmanHafez AbdelrahmanHafez added this to the 6.2.9 milestone Mar 25, 2022
@AbdelrahmanHafez AbdelrahmanHafez merged commit 3a797e9 into Automattic:master Mar 25, 2022
@chandiwalaaadhar chandiwalaaadhar deleted the patch-1 branch March 25, 2022 02:19
@AbdelrahmanHafez
Copy link
Collaborator

@Uzlopak
That's correct. We've come up with a workaround in #9010, which is giving an alias $on and using that alias internally, so users can use on if they want to.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Mar 25, 2022

Should we open a ticket to examine this issue?

@AbdelrahmanHafez
Copy link
Collaborator

@Uzlopak Done.
#11580

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This issue is due to a mistake or omission in the mongoosejs.com documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants