ClojureScript

timestamped source maps broken with Node

Details

  • Type: Defect Defect
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 1.9.293
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

I'm using Figwheel with Node, and noticed a bug with timestamped source maps. When the :source-map-timestamp compiler flag is set, the sourceMappingURL is set to source.js.map?timestamp.

This works fine in the browser, but breaks in Node where files are loaded from the filesystem. It looks like a simple workaround would be to check if :target is :node and output something like source.js.timestamp.map instead, and use it directly as the value of sourceMappingURL.

Here's a change I made locally in cljs.compiler/emit-source-map that allows source maps to be resolved on Node when using timestamps:

emit-source-map
(defn emit-source-map [src dest sm-data opts]
     (let [timestamp (System/currentTimeMillis)
           filename (str (.getPath ^File dest)
                         (when (and
                                 (true? (:source-map-timestamp opts))
                                 (= (:target opts) :nodejs))
                           (str "." timestamp))
                         ".map")
           sm-file  (io/file filename)]
       (if-let [smap (:source-map-asset-path opts)]
         (emits "\n//# sourceMappingURL=" smap
                (string/replace (util/path sm-file)
                                (str (util/path (io/file (:output-dir opts))))
                                "")
                (if (and (true? (:source-map-timestamp opts))
                         (not= (:target opts) :nodejs))
                  (str
                    (if-not (string/index-of smap "?") "?" "&")
                    "rel=" timestamp)
                  ""))
         (emits "\n//# sourceMappingURL="
                (or (:source-map-url opts) (.getName sm-file))
                (if (and (true? (:source-map-timestamp opts))
                         (not= (:target opts) :nodejs))
                  (str "?rel=" timestamp)
                  "")))
       (spit sm-file
             (sm/encode {(url-path src) (:source-map sm-data)}
                        {:lines                   (+ (:gen-line sm-data) 2)
                         :file                    (url-path dest)
                         :source-map-path         (:source-map-path opts)
                         :source-map-timestamp    (:source-map-timestamp opts)
                         :source-map-pretty-print (:source-map-pretty-print opts)
                         :relpaths                {(util/path src)
                                                   (util/ns->relpath (first (:provides opts)) (:ext opts))}}))))

Activity

Hide
David Nolen added a comment -

Does Node.js have source map caching issues? The timestamp feature was created for caching issues present in web browsers.

Show
David Nolen added a comment - Does Node.js have source map caching issues? The timestamp feature was created for caching issues present in web browsers.
Hide
Dmitr Sotnikov added a comment -

I tried it with :source-map-timestamp set to false, and source maps got out of sync when Cljs sources were reloaded.

Show
Dmitr Sotnikov added a comment - I tried it with :source-map-timestamp set to false, and source maps got out of sync when Cljs sources were reloaded.
Hide
David Nolen added a comment -

Okay. This issue will require more investigation into Node.js source mapping support before pursuing anything. As the behavior is understood, information should be added here.

Show
David Nolen added a comment - Okay. This issue will require more investigation into Node.js source mapping support before pursuing anything. As the behavior is understood, information should be added here.
Hide
Dmitr Sotnikov added a comment -

Sounds like a plan.

Show
Dmitr Sotnikov added a comment - Sounds like a plan.
Hide
David Nolen added a comment -

OK I took a look at the implementation of source-map-support, it does indeed cache the source map. However the proposed idea here isn't comprehensive enough. We need to change all the places where :source-map-timestamp is used in the source code. Patch is welcome.

Show
David Nolen added a comment - OK I took a look at the implementation of source-map-support, it does indeed cache the source map. However the proposed idea here isn't comprehensive enough. We need to change all the places where :source-map-timestamp is used in the source code. Patch is welcome.
Hide
Dmitr Sotnikov added a comment -

Yeah, I noticed the key is used in a few places. I can definitely take a look at making a patch in the near future if the approach looks good to you.

Show
Dmitr Sotnikov added a comment - Yeah, I noticed the key is used in a few places. I can definitely take a look at making a patch in the near future if the approach looks good to you.
Hide
Dmitr Sotnikov added a comment -

It looks like the approach of adding a timestamp introduces some problems. Generating unique file names would mean that old files have to be cleaned up somehow, since the new files will no longer overwrite them. Having to keep track of that isn't ideal. Perhaps it would be better to see if there's a way to prevent Node from caching the source maps.

Show
Dmitr Sotnikov added a comment - It looks like the approach of adding a timestamp introduces some problems. Generating unique file names would mean that old files have to be cleaned up somehow, since the new files will no longer overwrite them. Having to keep track of that isn't ideal. Perhaps it would be better to see if there's a way to prevent Node from caching the source maps.
Hide
Andrea Richiardi added a comment -

Hello folks I was exploring options for this and I went ahead and read the Source Map Options: it looks like a solution could be similar to what Meteor does, which is to load source maps in memory so that there would be no need to write files. We are in the REPL already so this should really be straightforward to implement, if I am not missing anything of course.

Show
Andrea Richiardi added a comment - Hello folks I was exploring options for this and I went ahead and read the Source Map Options: it looks like a solution could be similar to what Meteor does, which is to load source maps in memory so that there would be no need to write files. We are in the REPL already so this should really be straightforward to implement, if I am not missing anything of course.

People

Vote (1)
Watch (1)

Dates

  • Created:
    Updated: