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

Retire this package? #23

Open
chancancode opened this issue Sep 29, 2023 · 7 comments
Open

Retire this package? #23

chancancode opened this issue Sep 29, 2023 · 7 comments

Comments

@chancancode
Copy link
Member

The original intention of this package was to vendor and freeze the code as it was when we removed these components from Ember, and then just leave it mostly untouched. It seems like, between needing to tweak things in order for it to work outside of core, and ongoing maintenance to keep up with ecosystem changes, the plan didn't work out as well as we had hoped. I was going to publish a new version but it appears that things (lint, types, etc) are in a broken state on main. In the meantime I published an alpha version as a compromise.

Is it time to retire this package? As far as components are concerned, these components aren't that complicated and there isn't a ton of code. At this point it would probably be better for those who still need it to vendor their own version that they can tweak as much as they need?

Thoughts?

@SergeAstapov
Copy link
Contributor

The only reason IMO this may be needed, it's used in ember-engines and there is no direct replacement.

We could copy-paste needed code for <LinkTo /> component to ember-engines` and drop dependency on this addon though.

@chancancode
Copy link
Member Author

I think that would probably be better. Engines need to customize it anyway so they may as well do it more directly and maintain their own tests. The tests from ember was supposed to be carried over but didn’t end up making it. So anyone using this package is using untested code. If the code was actually frozen you could make the argument that it’s “safe by proxy of having existed for so long”, but that aren’t even true anymore

@chancancode
Copy link
Member Author

I also don’t know that engines actually need to support the whole suite of legacy api in the link component (classNames, tagName and all that). I think really people’s expectations is a version of LinkTo that works like the one in Ember but works with engines. At this point they should just do a breaking change release and align with the LinkTo api in recent Ember imo

@SergeAstapov
Copy link
Contributor

@chancancode sounds like a plan! I'll try to work on that

@chancancode
Copy link
Member Author

If you are planning on it being a breaking change anyway, you may want to make it a @glimmer/component and base the code off of https://github.com/emberjs/ember.js/blob/main/packages/%40ember/-internals/glimmer/lib/components/link-to.ts instead of the version vendored here. Ember core couldn't use the actual glimmer components and tracked properties (at the time at least), but you can, so you can probably make it much simpler

@chancancode
Copy link
Member Author

(You should also decide if you really need to support other random things like @current-when 🤷🏼)

@void-mAlex
Copy link
Contributor

@chancancode I agree this package should be retired
I also think in the interest of trying to keep as many people in a 'working state' we should still release a few cleanup prs
some changes from me have already been merged that will immediately help anyone using engines with embroider especially
happy to put forward a plan for this + engine work and act on that to get both packages in a better state especially for embroider world and less wacky for the rest of the ecosystem upgrading to ember-source versions

roughly what I see left on here would be to add guidance when we detect you're on an engines version that doesn't need this anymore and try and get people to remove the package

for now if we could release a major with the landed changes then I can progress work on engines side to get that lined up and come back here with a minor release after to get it ready for retirement

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

No branches or pull requests

3 participants