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

Support for class aliases #5873

Closed
wants to merge 3 commits into from
Closed

Support for class aliases #5873

wants to merge 3 commits into from

Conversation

Taluu
Copy link
Contributor

@Taluu Taluu commented Nov 17, 2016

Sometimes, some class names are changed (as it was reported in #4043), but not necessarly for a version below 1.0, where BC is not that important.

I encountered a use case while trying to use BrainTree, where they migrated all their pear style names classes into namespaces... But they added class_alias so that their classes are still "pear-loadable". They didn't want to update their docs ("because it shows that the old way still works, which I think is BS), and it posed problems when you use the authoritative classmap, as their aliases are loaded in the class file declaration (and on authoritative, Braintree_Configuration for example is not found, because the alias is not declared yet).

So, to be back to the point, having composer handle this would be I think a nice to have ; as @Seldaek said, they could have given a file function that declares all these aliases, but IMO it is not a good solution, as this would trigger all the autoloading of the classes when you may not need those...

WDYT ?

@Taluu
Copy link
Contributor Author

Taluu commented Nov 17, 2016

Note ; for the brain tree issue, see braintree/braintree_php#154

@Seldaek
Copy link
Member

Seldaek commented Nov 17, 2016

IMO this shouldn't be composer's problem, if people want to define aliases then ok, if that restricts you from using an authoritative classmap then you can update your code.. These are all choices as far as I can tell. If we aren't blocking users per se, I'd rather not add a new feature for it.

@stof
Copy link
Contributor

stof commented Nov 17, 2016

if the PSR-0 location allows to find the files for the old class names (which contain the class alias), we should update our optimized classmap generator to detect this class_alias usage and add it in the classmap too IMO. This would be the clean solution for the autoritative classmap, without requiring adding new features (and without requiring any change in braintree itself except updating composer)

@Seldaek
Copy link
Member

Seldaek commented Nov 17, 2016

That would mean parsing the whole code if we wanna do it right.. a whole new can of worms just to support class aliases which really isn't a thing that causes issues to a lot of people. Authoritative classmap is an aggressive optimization that comes with a few caveats.. If anything we need to document them better I'd say, but I think we can live with the fact that it doesn't apply to every project out there.

@nicolas-grekas
Copy link
Contributor

nicolas-grekas commented Dec 6, 2016

There is another situation where this wouldn't work, even doing what @stof suggested: checking types with e.g. instanceof. Aliasing must be done in the same file that declares the class. For authoritative maps, one could return before a dummy class declaration that would satisfy the current parser.
All in all, this may be the way to espace from pear style namespaces in a smooth way!

@Seldaek Seldaek closed this Jan 22, 2017
@ebuildy
Copy link

ebuildy commented Oct 23, 2019

In our case, we are migrating a big project to SF4 and the src code must still work with SF2, so I upgraded DoctrineMongoDB and added in Symfony Kernel:

class_alias('Doctrine\ODM\MongoDB\Repository\DocumentRepository', 'Doctrine\ODM\MongoDB\DocumentRepository');

Maybe composer could handle it, with a deprecation notice

@Taluu Taluu deleted the class-aliases branch October 23, 2019 15:00
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.

None yet

5 participants