Namespacing changes due to new class loader in Discourse

@members


Rails 6 introduced a new class autoloader called Zeitwerk which will give us some dev performance boost.

What’s important to note is, the new loader is stricter so we’ll need to fix some namespaces in our plugins in order for them to function smoothly.

Also, We’ll probably be able to remove the require/require_dependency calls.

For reference, you guys can refer the above link where a team member has updated all the namespaces in all the official plugins.

2 Likes

Thanks @fzngagan

I’ll do a review of all plugins apart from Events and TLP tomorrow.

1 Like

I’ll check with Events today.

I just checked and its a non issue for events. Thanks to you @angus as you’ve already made sure that classes use the top level namespace. Altough, many of other plugin authors will have to fix the issue. I got errors due to other plugins

Also, here is a new rake task to make our lives easier.

rake zeitwerk:check   # Checks project structure for Zeitwerk compatibility

I’ve just rebuilt rideables with my fork and previews are not generating after rebuilding a post.

The site doesn’t break though and otherwise everything is normal.

That’s a big issue though and will need resolving.

1 Like

It would be awesome if you share the changes you make in the process.

1 Like

Certainly will. The weird part is there is no error in the logs.

1 Like

Yeah. That shouldn’t be the case as this change affects only the Rails side of the things.

What seems to be broken is one-boxing. I’m going to try to rebuild without the ‘onebox assistant’ plugin and retest. There are some risky over-bearing overrides in that plugin and its been a while since I refreshed the code from core so it could be the source of the issue.

1 Like

Yep that was the issue! I think that plugin is now broken. TLP looks good atm but not tested it exhaustively

1 Like

(Actually makes sense, if the namespacing has changed it’s probably no longer overriding the right things!)

Namespacing hasn’t changed. The way Namespaces are looked up has changed. So if you want to make the autoloader lookup the parent namespace you need to add:: to the class name.

Yeah, that might be happening accidentally. Otherwise, it would be an error.

1 Like

Updated OneBox Assistant. It appears to be working again:

1 Like

Hm, I wonder if we should remove

require_dependency

Considering some sites will want to update the plugin without updating Discourse. Thoughts?

1 Like

I had the temptation to do that in events, but realized its a bit early to do so.

1 Like

Hmmm. This could impact a few:

Jobs -> ::Jobs

eg I suspect Potus will need this tweak when upgraded.

1 Like