ClojureScript

clj->js trims the namespace prefix from keywords while writing them to string

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Declined
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:

Description

The following behavior was observed and confirmed from the code:

(clj->js :ns/n) => "n"

I believe this is a limitation and the namespace of the keyword should be kept while writing it to string.
The code in core.js does this while handling keywords:

(keyword? x) (name x)

while it should do this (or something similar):

(keyword? x) (str (namespace x) "/" (name x))

Activity

Hide
Vasile added a comment -

a better (working) fix: (keyword? x) (str (if (namespace x) (str (namespace x) "/")) (name x))

Show
Vasile added a comment - a better (working) fix: (keyword? x) (str (if (namespace x) (str (namespace x) "/")) (name x))
Hide
David Nolen added a comment -

I'd be willing to take a patch that allows this behavior to be configured.

Show
David Nolen added a comment - I'd be willing to take a patch that allows this behavior to be configured.
Hide
Niklas Närhinen added a comment -

Proposed fix

Show
Niklas Närhinen added a comment - Proposed fix
Hide
Niklas Närhinen added a comment -

Fixed version of patch

Show
Niklas Närhinen added a comment - Fixed version of patch
Hide
David Nolen added a comment -

Excellent, Niklas I don't see you on the list of contributors, please send in your Contributor Agreement, http://clojure.org/contributing, so we can apply the patch.

Show
David Nolen added a comment - Excellent, Niklas I don't see you on the list of contributors, please send in your Contributor Agreement, http://clojure.org/contributing, so we can apply the patch.
Hide
David Nolen added a comment -

I looked more closely at the clj->js source, the system is already customizable. We can't determine ahead of time how you might want to emit keywords and symbols - thus you can extend Keyword and Symbol to IEncodeJS yourself and get the desired behavior.

Show
David Nolen added a comment - I looked more closely at the clj->js source, the system is already customizable. We can't determine ahead of time how you might want to emit keywords and symbols - thus you can extend Keyword and Symbol to IEncodeJS yourself and get the desired behavior.
Hide
Andrea Richiardi added a comment -

I have just stumbled across this one, shall we at least say it in the docstring of clj->js that we are losing the namespace part?

Show
Andrea Richiardi added a comment - I have just stumbled across this one, shall we at least say it in the docstring of clj->js that we are losing the namespace part?
Hide
Jozef Wagner added a comment -

With the introduction of specs, the namespaced keywords are being used more and more. This issue prevents streamlined edn->json->edn transformation. I think it should be reopened. IMO the 'lossy' method should never be a default one.

Show
Jozef Wagner added a comment - With the introduction of specs, the namespaced keywords are being used more and more. This issue prevents streamlined edn->json->edn transformation. I think it should be reopened. IMO the 'lossy' method should never be a default one.
Hide
Paulus Esterhazy added a comment -

Unless we are willing to break existing code, I don't think it will be possible to change the default behavior.

I'm also not sure that extending IEncodeJS is the best solution, as it affects every call to clj->js, including calls to libraries which may rely on the ns-stripping behavior.

However, the attached patch allows you to make the decision on a per-call basis.

One quibble with the patch: perhaps it would be better to use kwargs style `(clj->js v :preserve-namespaces true)` in line with `js->clj`?

Show
Paulus Esterhazy added a comment - Unless we are willing to break existing code, I don't think it will be possible to change the default behavior. I'm also not sure that extending IEncodeJS is the best solution, as it affects every call to clj->js, including calls to libraries which may rely on the ns-stripping behavior. However, the attached patch allows you to make the decision on a per-call basis. One quibble with the patch: perhaps it would be better to use kwargs style `(clj->js v :preserve-namespaces true)` in line with `js->clj`?

People

  • Assignee:
    Unassigned
    Reporter:
    Vasile
Vote (0)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved: