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

[DO NOT MERGE] Use lazily-initialized shims for history singletons #3388

Closed
wants to merge 1 commit into from

Conversation

taion
Copy link
Contributor

@taion taion commented Apr 26, 2016

Maybe?

It'd address e.g. #3387.

Going to make a separate PR to history that makes the base href check lazy, which is probably more correct.

@taion
Copy link
Contributor Author

taion commented Apr 26, 2016

This is probably more correct: remix-run/history#283

@taion taion changed the title Use lazily-initialized shims for history singletons [DO NOT MERGE] Use lazily-initialized shims for history singletons Apr 26, 2016
@codecov-io
Copy link

Current coverage is 95.01%

Merging #3388 into master will not change coverage

@@             master      #3388   diff @@
==========================================
  Files            30         30          
  Lines           921        921          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits            875        875          
  Misses           46         46          
  Partials          0          0          

Sunburst

Powered by Codecov. Last updated by e5bcee2

@taion
Copy link
Contributor Author

taion commented Apr 26, 2016

Erm. wat

@timdorr
Copy link
Member

timdorr commented Apr 26, 2016

ALL HAIL THE GIANT GREEN CIRCLE OF COVERAGE!

'createPath',
'createHref',
'createLocation'
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wat.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really don't like this. I get that we're tied to the API internally anyways, but this seems like an antipattern to me. I would kill for a method_missing equivalent in JS, but that's not going to happen until Proxy is more prevalent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Poor man's Proxy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the right way to go about it anyway, the history PR is much better.

@timdorr
Copy link
Member

timdorr commented Apr 27, 2016

history 2.1.1 is out, so this shouldn't be needed.

@timdorr timdorr closed this Apr 27, 2016
@taion taion deleted the history-shims branch April 27, 2016 20:52
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants