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

Removal of link cache breaks the test example in the README #716

Open
jeffhertzler opened this issue Jun 17, 2022 · 3 comments
Open

Removal of link cache breaks the test example in the README #716

jeffhertzler opened this issue Jun 17, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@jeffhertzler
Copy link

jeffhertzler commented Jun 17, 2022

Because the cache is removed the link created with linkFor and the link in the template no longer match which causes the test to fail by trying to redirect the browser.

Repro: https://github.com/jeffhertzler/ember-link-repro

I know that 2.0.0 was marked as breaking and this was somewhat expected, but the example is wrong now and it's not clear to me exactly how you would write this kind of test anymore.

@buschtoens
Copy link
Owner

Broken as of #658.

Thanks for providing a repro! And sorry, that this has hit you.

I'll try to think of a solution, but I'll happily accept ideas / PRs as well.

@buschtoens buschtoens added the bug Something isn't working label Jun 24, 2022
@enspandi
Copy link

enspandi commented Jul 8, 2022

In my case, I'm overriding the helper like this

this.owner.unregister('helper:link');
this.owner.register(
  'helper:link',
  class extends Helper {
    compute(positional: LinkHelperPositionalParams, named: LinkHelperNamedParams) {
      return linkFor(named.route, named.models);
    }
  }
);

And have my own linkFor function that manages a cache... not the best solution probably, but it's not much code either. Good enough for integration tests.

@jeffhertzler
Copy link
Author

Is enabling the cache in the library only in test mode a viable solution? It looks like the cache was removed to make support for ember 4 work (maybe that's already how it worked?). is there an alternative path to adding the cache back that doesn't rely on using fast-json-stable-stringify if it's causing issues? Also, at first glance it seems like onTransitionTo and onReplaceWith don't need to factor into the cache key. I don't know if these are viable suggestions. I'll think about it and familiarize myself with the codebase a bit more.

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

No branches or pull requests

3 participants