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

MediaCloud Plugin overrides Illuminate collapse() function #15

Closed
conradfuhrman opened this issue Feb 6, 2021 · 10 comments · Fixed by #16
Closed

MediaCloud Plugin overrides Illuminate collapse() function #15

conradfuhrman opened this issue Feb 6, 2021 · 10 comments · Fixed by #16
Assignees
Labels
bug Something isn't working

Comments

@conradfuhrman
Copy link

Reaching out as I'm not sure how to solve this one. MediaCloud's (https://mediacloud.press/) uses similar Illuminate classes that define the collapse() method and it seems to be overriding Poet's use of collapse() directly from the Illuminate vendor. I've reached out to MediaPress to see if this is something they can resolve, but haven't heard back. I was curious if this is something that is a scoping issue or similar. I've actually never run into this before.

Of course, I'm happy to help resolve, test, etc., but this is a bit out of my comfort zone on where a solution might be. Appreciate your work and really want to give Poet a try!

@Log1x
Copy link
Owner

Log1x commented Feb 6, 2021

hmm strange. is it throwing an error?

@conradfuhrman
Copy link
Author

No explicit error, but when MediaPress is disabled everything works awesome. When it's enabled I did a var_dump on the collection chained methods and collapse returns an empty array rather than the proper response of the post types.

The method from MediaPress is slightly different so that's where my assumption lies with their class.

So technically it's a soft failure. Also this is a brand new Sage 10 project and MediaPress is currently the only plugin other than ACF.

@Log1x Log1x self-assigned this Feb 6, 2021
@Log1x
Copy link
Owner

Log1x commented Feb 6, 2021

Confirmed the issue. Will try to push a fix tonight.

@Log1x Log1x added the bug Something isn't working label Feb 6, 2021
@conradfuhrman
Copy link
Author

That's amazing! Looking forward to seeing how you resolved it and thanks a ton for providing a fix.

@conradfuhrman
Copy link
Author

Going to test tonight the newest version 4.2.5 as it looks like a lot updated from MediaPress. Will let you know if the issue still persists.

@conradfuhrman
Copy link
Author

Tested with 4.2.5. Still have the same issue unfortunately.

@Log1x
Copy link
Owner

Log1x commented Feb 11, 2021

yeah. annoyingly they are autoloading helpers.php from their old Illuminate\Support\Collection and it's registering the collect() helper before Sage's. I fixed it on https://github.com/Log1x/poet/pull/16/files but it's not stable yet.

its likely for inconsistencies like this to show up elsewhere down the road unless they were to fix registering their custom collect in the global namespace.

@conradfuhrman
Copy link
Author

Gotcha. Anything I can do to help?

@Log1x
Copy link
Owner

Log1x commented Feb 11, 2021

If you have time to burn, feel free to try that PR so far. I will have to swing back around to it tonight – I honestly can't remember where I left off. :(

@conradfuhrman
Copy link
Author

Sounds good, I'll fork it and see what I come up with in the next day or two.

@Log1x Log1x mentioned this issue Feb 17, 2021
@Log1x Log1x closed this as completed in #16 Feb 17, 2021
Log1x added a commit that referenced this issue Feb 17, 2021
* BREAKINGCHANGE(config): Rename menu to adminMenu
* BREAKINGCHANGE(config): Rename `categories` to `blockCategories`
* BREAKINGCHANGE(anchors): Anchors have been deprecated from Poet. If you happen to use them and see this, hit me up – I wouldn't mind whipping up a separate package.
* enhance(poet): Split Poet into modules (Fixes #10)
* enhance(poet): Refactor and improve code of project
* chore(deps): Bump dependencies
* enhance(poet): Move Collection into a trait to avoid using the global `collect()` function (Fixes #15)
* enhance(admin-menu): Rewrite Admin Menu module to not use `$GLOBALS`
* enhance(admin-menu): Allow configuring the admin page slug when moving admin menu items
* chore(ci): Move to GitHub actions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants