Move serialization to own module

Dumb question, but does serialization have a use outside of JSON-LD? And if not, could we just add it to that module? I'm growing a bit concerned at the amount of nonsensical choices we're throwing at site builders in D8 as a result of trying to modularize everything.

If that doesn't make any sense, feel free to proceed.

Or I guess put another way, why does this need to be a checkbox in the user interface at all? Why can't it be lazy-loaded when something in the system needs serialization? I thought that was kinda the point of all this OO / Symfony stuff.

+++ b/core/modules/serialization/lib/Drupal/serialization/RegisterSerializationClassesCompilerPass.phpundefined
@@ -2,10 +2,10 @@
+namespace Drupal\serialization;

@@ -14,7 +14,7 @@
+class RegisterSerializationClassesCompilerPass implements CompilerPassInterface {

Should this be in it's own namespace? not sure.

+++ b/core/modules/serialization/lib/Drupal/serialization/SerializationBundle.phpundefined
@@ -0,0 +1,39 @@
+ * Contains Drupal\rest\SerializationBundle.

\ suffix and correct module namespace.

+++ b/core/modules/serialization/lib/Drupal/serialization/SerializationBundle.phpundefined
@@ -0,0 +1,39 @@
+ * Serialization dependency injection container.

Not really about this issue, but not sure why we use this comment :/ I think all the bundle classes use it but it doesn't sound right.

Also, if these formats are moving to their own module, I think #1897612: Add YAML support to serialization module should go in too.

@webchick: Yeah, the container will lazy load ALL THE THINGS, I guess it's about not putting lots of things in the container that don't need to be there. I guess the overhead will be practically nothing, I see it more of a housekeeping thing :)

No problem! The changes look good. I also have no strong opinion about the namespace, so leaving it is fine by me.

Having a separate module does make alot of sense in general. Lin's point in #6 regarding this is a good one.

RTBC from me. I assume this will pass, as the last failures were random ones (sigh).

Sorry! There was one rogue '\' that needed fixing :) This is def still RTBC.

Hmm I'm not entirely sure about a whole new module for this either. Is it worth looking at whether we can have a single serialization service similar to the plugin managers?

Generally these seem like very optional services for a site to use (as opposed to something like cache or locking which is required for Drupal to function at all), so it didn't feel right having them on CoreBundle - that's sort-of the new system.module.

Related to #16, there's a couple issues. One is that currently, non-module plugins are required to be in Drupal\Core\Plugin rather than Drupal\Core\SUBSYSTEM\Plugin. #1828616: AnnotatedClassDiscovery is not finding plugins in Drupal\(Core|Component)\COMPONENT_NAME\Plugin directory is the issue hashing that out, but in the meantime, we actually recently committed some validation plugins to Drupal\Core\Plugin\Validation\*, so this limitation is more of an annoyance than a hard-blocker.

However, in addition to the solvable plugin annoyance, we also currently require both configuration files themselves, and ConfigEntity definitions that other modules can supply instanced config files for, to reside within modules. And based on the discussion and work so far in #1880424: Handle entity references on import, it seems to be likely that some configuration will be needed related to Serialization, particularly related to configuring how entity references in URI/UUID/etc. format are resolved.

I suppose it's possible we'll find some solution to that issue that doesn't require any configuration. If we do require configuration though, should we put it in system.module, or would a serialization.module make sense? If system.module, we should reroll #1880424: Handle entity references on import accordingly and proceed that way. If serialization.module, then I think it would be helpful to commit this issue separately, to make the other one easier to review and iterate. And perhaps we could add a "revisit before release" tag here in order to move it back into a non-module component if it turns out we didn't need any Serialization configuration in that issue or any other remaining web services work.

Just a no-op reroll of #9 cause that one didn't apply.

I chatted more with Lin today about #1880424: Handle entity references on import. She's working on some new ideas in that issue, but we're pretty confident that some administrator configuration will be required. For example, we can ship with UUID and URI reference handlers, and the URI one could be made to actually fetch the corresponding resource or not, and it makes sense for an administrator to be able to configure whether that's desired or not.

So, I think it makes sense for Serialization to be a module. It's not just a generic serialization service. It's specifically, serialization for Drupal, which involves entities, which involves entity references, which requires configuration.

Therefore, RTBC, though open to other suggestions, such as putting the configuration-related stuff in system.module instead, or leaving base services in Drupal\Core\Serialization, and having a serialization.module only for configuration-related stuff.

Is it worth looking at whether we can have a single serialization service similar to the plugin managers?

Possibly, but I think that should be a separate issue.

Hm. Ok, let's do this for now, but tagging so we can go back and revisit this, since sometimes these "temporary" workarounds... aren't so temporary. ;D

Committed and pushed to 8.x. Thanks!

Time to revisit. Still not sure on this one.

Discussed with @effulgentsia. So serialization was moved into a module because:

1. It was cluttering up the container.
2. As an alternative to the clutter, there was a suggestion of using a single serialization service, similar to the plugin manager, but plugins could not yet be provided by core.
3. There was a possibility that #1880424: Handle entity references on import would require configuration to resolve, and only modules could provide config.

The blocker for #2 has been resolved now, so we could resolve #1 with #2. #3 is apparently not an issue because it appears #1880424: Handle entity references on import was resolved without config.

Does it make sense to open a followup issue to create the single serialization manager service and move it back out of a module? Alternately, we could just live with it in a module. @effulgentsia wasn't sure such a followup would need to be beta deadline, but I think it might; a module disappearing from your site in a beta update seems like it could cause problems.

The cluttering up the container argument seems to get ignored most of the time form my experience, I brought that up a few times, to no avail.

How would the single serialization service work exactly? Would there be plugins for encoders/decoders and then normalizers? If so, would the plugins be in charge of their own priority, as that is a very important aspect when it comes to normalizing stuff.

I don't see too much of an issue, or much gain from moving this back out into Core tbh. I don't feel too strongly either way. I'm just interested in how you (and others) see a serialization manager working with regard to the questions above.

I have no idea how it would work actually; I was just trying to summarize the issue since we need to either change it before beta or live with it. :) I'm also not too bothered by it being in a module.

I feel it is probably best to leave it alone as a module for now in the interest of getting Drupal 8 out sooner. The benefit of getting the beta/release out is much bigger than the cost of doing this cleanup. There is no reason this can't wait until Drupal 9 and can't see it being worth doing at this time.

We need to consider moving this to 9.x as a new issue.

I'm down with that. Its not even really clean up to be honest.

I'd like to respectfully disagree with that decision.

Serialization module is bad for UX: It's an option that clutters the UI. You can enable the option. But it does not do anything.

Serialization module is bad for DX: It's just some library code to bridge from Symfony Serializer to REST module. It is unlikely that any other code than REST module will depend on it. Developers are repetitively asking why this is an own module.

At minimum, it would be wise to merge Serialization module into REST module.

I hear your points, this is a good reason to make this just a component of core. It is indeed a bridge/api module of sorts.

However, I do not agree that this should be merged into the rest module.