Skip to content

Instantly share code, notes, and snippets.

@Deraen
Last active July 5, 2017 12:31
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Deraen/9cf7d04221450bc10ac571399a483c4d to your computer and use it in GitHub Desktop.
Save Deraen/9cf7d04221450bc10ac571399a483c4d to your computer and use it in GitHub Desktop.
CLJS-2143

Option 1.

https://github.com/Deraen/clojurescript/commit/bb94e566a8827d0356740491e2506fba02faa109

  • Load all js_transform.clj files in classpath
  • Problems:
    • files are not real namespaces (no ns)
    • load-reader will eval the code in file each time load-js-transforms! is called, which is each time cljs.closure/build is called
    • a js_transform.clj file is loaded even if js-transforms method from it is not needed
      • for example, if any lib in classpath has dep on cljsjs/babel-standalone

Option 2.

https://github.com/Deraen/clojurescript/commit/9866a33c78ebf882e5e01d07316d00c7794e8edc

  • If js-transforms method is not found, but the value is namespaced keyword, try to load namespace pointed by the namespace part of the keyword and run js-transforms again in hopes that the namespace provided the method
  • If namespace doesn't provide the method, there will be a warning about unsupported preprocess value
  • Benefits vs. Option 1:
    • Real namespaces
    • Works with current cljsjs/babel-standalone
    • Minimal change

Option 3?

  • :preprocess value should be a symbol
  • js-transforms multimethod is removed
  • symbol will point directly at a function, the namespace is required if needed
  • Benefits:
    • Using a symbol to refer to a function is better than refering to a multimethod using namespaced keyword

Other ideas

  • :preprocess should be allowed to be vector of transformations to run?
    • Would be handled at process-js-modules, will work with options 2 and 3
@swannodette
Copy link

swannodette commented Jul 5, 2017

Option 3 sounds good to me. I don't think we need to remove js-transforms immediately to proceed though. Supporting vectors sounds OK but it seems like an easy thing to add after the main work is done. @thheller I'm not seeing the value of the interceptor model here, since that was done for asynchrony. Is there something you have in mind?

@Deraen
Copy link
Author

Deraen commented Jul 5, 2017

It should be easy to leave the multimethod version around and use that when the value is keyword).

I agree that vector support can be added later, if needed.

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