Skip to content

src: add node::url::URL::href() method #22610

Closed
Closed
src: add node::url::URL::href() method#22610
Labels
feature requestIssues requesting new Node.js features.help wantedIssues that need assistance from volunteers or PRs that need help to proceed.whatwg-urlIssues and PRs related to the WHATWG URL implementation.

Description

@alexkozy
alexkozy
opened on Aug 31, 2018

Is your feature request related to a problem? Please describe.
I need to get href based on instance of URL object in native code.

Describe the solution you'd like
I can use href() method that is added in addition to existing getters.

Activity

  1. alexkozy
    added
    help wantedIssues that need assistance from volunteers or PRs that need help to proceed.
    lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.
    on Aug 31, 2018
  2. addaleax
    added
    whatwg-urlIssues and PRs related to the WHATWG URL implementation.
    and removed
    lib / srcIssues and PRs involving general changes in the lib/ or src/ directories.
    on Sep 3, 2018

  3. addaleax commented on Sep 3, 2018

    @addaleax
    addaleax
    on Sep 3, 2018
    Member

    @nodejs/url

  4. devsnek commented on Sep 3, 2018

    @devsnek
    devsnek
    on Sep 3, 2018
    Member

    the url serializer is written in js, so i'm not 100% sure if this is possible without a lot of code duplication

  5. TimothyGu commented on Sep 3, 2018

    @TimothyGu
    TimothyGu
    on Sep 3, 2018
    Member

    @devsnek Yeah my guess is that we will have to duplicate. That's fine though, as we already have a duplicated URL class in C++ anyway.

  6. tigercosmos commented on Sep 15, 2018

    @tigercosmos
    tigercosmos
    on Sep 15, 2018

    Are there any specs for URL::href()?

  7. tigercosmos commented on Sep 15, 2018

    @tigercosmos
    tigercosmos
    on Sep 15, 2018

    I am not sure if is this one
    https://url.spec.whatwg.org/#dom-url-href

  8. TimothyGu commented on Sep 15, 2018

    @TimothyGu
    TimothyGu
    on Sep 15, 2018
    Member

    @tigercosmos Yes! You can even look at how lib/internal/url.js implements it in JavaScript, and just duplicate that logic in C++.

  9. WaleedAshraf commented on Oct 12, 2018

    @WaleedAshraf
    WaleedAshraf
    on Oct 12, 2018
    Contributor

    Hi @ak239
    I think you can use toString() also?
    They both return the same result.

  10. stropitek commented on Oct 12, 2018

    @stropitek
    stropitek
    on Oct 12, 2018
    Contributor

    I'm taking over this issue

  11. addaleax commented on Oct 12, 2018

    @addaleax
    addaleax
    on Oct 12, 2018
    Member

    @stropitek As @WaleedAshraf commented, this might not be necessary since ToString() also works… just as a heads up, so you don’t do unnecessary work :)

  12. davesters commented on Oct 12, 2018

    @davesters
    davesters
    on Oct 12, 2018
    Contributor

    May be worth it to dig deeper to verify if ToString() actually exists and works in C++.

    If it does to remove this TODO and update to use the ToString() function.
    https://github.com/ak239/node/blob/master/src/inspector_agent.cc#L622

  13. stropitek commented on Oct 12, 2018

    @stropitek
    stropitek
    on Oct 12, 2018
    Contributor

    No the ToString method exists on URLHost class not on the URL class.

  14. Trott commented on Nov 13, 2018

    @Trott
    Trott
    on Nov 13, 2018
    Member

    Is this being actively worked on?

  15. Trott
    added
    feature requestIssues requesting new Node.js features.
    on Nov 18, 2018

  16. Trott commented on Nov 18, 2018

    @Trott
    Trott
    on Nov 18, 2018
    Member

    @ak239 This is still something you want/need? Or did you find some other workaround?

  17. WaleedAshraf commented on Nov 18, 2018

    @WaleedAshraf
    WaleedAshraf
    on Nov 18, 2018
    Contributor

    @Trott
    @addaleax and I looked into this on Code + Learn. ToString() method can serve the purpose. I think this can be closed.

  18. targos commented on Nov 18, 2018

    @targos
    targos
    on Nov 18, 2018
    Member

    There is no URL::ToString method and AFAIK @stropitek has a wip branch to add it

  19. WaleedAshraf commented on Nov 18, 2018

    @WaleedAshraf
    WaleedAshraf
    on Nov 18, 2018
    Contributor

    Yup. That's URLHost not URL. We were confused.

  20. Trott commented on Nov 24, 2018

    @Trott
    Trott
    on Nov 24, 2018
    Member

    There is no URL::ToString method and AFAIK @stropitek has a wip branch to add it

    Would it make sense to open a WIP PR?

  21. Trott commented on Apr 22, 2020

    @Trott
    Trott
    on Apr 22, 2020
    Member

    This seems like it stalled. I don't know if @ak239 still needs this after all this time and I don't know that anyone else is asking for it. I'd be inclined to close it unless @ak239 or someone else indicates it is still a desirable feature for them and/or someone opens a PR.

  22. alexkozy commented on May 26, 2020

    @alexkozy
    alexkozy
    on May 26, 2020
    MemberAuthor

    As far as I remember the motivation behind this one was driven by one of the comments in the code review for one of my PRs: #22251 (comment)
    It looks like there were no other requests for href getter over the last year and a half so I will close this one. Feel free to reopen it if you think that we need href getter.

  23. alexkozy
    closed this as completedon May 26, 2020
  24. watilde
    added a commit that references this issue on Nov 21, 2020

    src: integrate URL::href() and use in inspector

    e40ed28
  25. codebytere
    added a commit that references this issue on Nov 22, 2020

    src: integrate URL::href() and use in inspector

    6d1b1c7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues requesting new Node.js features.help wantedIssues that need assistance from volunteers or PRs that need help to proceed.whatwg-urlIssues and PRs related to the WHATWG URL implementation.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions