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

MONGOID-5210 Prefer field :type as Symbol + custom field type symbols #5119

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

johnnyshields
Copy link
Contributor

@johnnyshields johnnyshields commented Jan 1, 2022

This is blocked until after #5269 is merged.


Fixes MONGOID-5210

Prior to this PR, Mongoid already supports defining field :type as a Symbol rather than a Class:

  field :first_name, type: :string  # works today, even without this PR!

Generally speaking, it is much better to to use Symbol than Class, because Class uses autoloading etc.

This PR adds the following:

  • Main documentation is updated to prefer using Symbol rather than Class. I haven't done this change in 100% of doc files; I plan to do that after this PR is merged in a separate PR.
  • Usage of Class is deprecated and now logs a warning. Usage of Class is intended to be removed in Mongoid 8.0.
  • It is now possible to define custom symbols using the following:
  Mongoid::Fields.configure do
    type :point, Point
  end

Further Plans

In Mongoid 8.1+, once everything is symbols we should consider extracting out the mongoization logic into separate modules and not inserting methods into Ruby Kernel classes. For example, String.mongoize should be Mongoizers::String.mongoize(string).

@johnnyshields johnnyshields changed the title Prefer field :type as Symbol + custom field type symbols [WIP] Prefer field :type as Symbol + custom field type symbols Jan 1, 2022
@johnnyshields johnnyshields changed the title [WIP] Prefer field :type as Symbol + custom field type symbols MONGOID-5210 [WIP] Prefer field :type as Symbol + custom field type symbols Jan 1, 2022
@johnnyshields johnnyshields changed the title MONGOID-5210 [WIP] Prefer field :type as Symbol + custom field type symbols MONGOID-5210 Prefer field :type as Symbol + custom field type symbols Mar 8, 2022
…Symbol is already supported today.)

- Deprecate using field type as a Class.
- Add ability to define custom field types using a mini DSL (Mongoid::Fields.configure)
- Fix Mongoid::Fields.option documentation
@johnnyshields
Copy link
Contributor Author

@p-mongo tests are now written. This is ready for final review and merge.

Copy link
Contributor

@p-mongo p-mongo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @johnnyshields ,

The portion of this PR that we can merge is the one described in https://jira.mongodb.org/browse/MONGOID-5336 which is permitting applications to define their own aliases. Can you please remove from the diff the changes that have to do with replacing class use with symbol use?

@johnnyshields
Copy link
Contributor Author

johnnyshields commented May 9, 2022

@p-mongo I will be happy to split out that code soon. However, please see my reply in https://jira.mongodb.org/browse/MONGOID-5210, in the future, I still believe there is strong justification to switch to symbols.

@johnnyshields
Copy link
Contributor Author

@p-mongo PR split out, please see: #5269

@johnnyshields johnnyshields marked this pull request as draft June 25, 2022 08:19
@johnnyshields johnnyshields marked this pull request as ready for review January 26, 2023 17:49
@johnnyshields johnnyshields changed the title MONGOID-5210 Prefer field :type as Symbol + custom field type symbols [READY FOR REVIEW] MONGOID-5210 Prefer field :type as Symbol + custom field type symbols Feb 10, 2023
@jamis jamis changed the title [READY FOR REVIEW] MONGOID-5210 Prefer field :type as Symbol + custom field type symbols MONGOID-5210 Prefer field :type as Symbol + custom field type symbols Apr 15, 2024
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.

3 participants