Fork me on GitHub
#cljs-dev
<
2018-03-30
>
dnolen12:03:49

I’m realizing that the work we did modules should probably always be true if your :target is Node.js

dnolen12:03:07

i.e. we shouldn’t bother transforming classpath ES6/CommonJS

dnolen12:03:32

it’s going to be a lot cleaner

dnolen12:03:54

will probably add a troubleshooting section to the end

thheller19:03:24

@jannis :pseudo-names true is your friend when debugging anything related to :advanced

jannis19:03:38

Ah! I’ve used that before but it’s been ages.

jannis19:03:50

Uncaught TypeError: (0 , $_ponyfill2$$module$Users$jannis$Work$oss$jannis$groom$node_modules$symbol_observable$lib$index$$.default) is not a function. I think https://gist.github.com/Jannis/fc1af47700279a3054ff1ca047b8fa7a (original and compiled filles) should give me enough to debug some.

jannis19:03:02

If anyone has seen this before, let me know 🙂

thheller20:03:51

@jannis this might be an issue related to how closure rewrites babel processed ES6 files

thheller20:03:57

the namespace USING this file probably used import Thing from "symbol-ovservable"

thheller20:03:18

this does not work since the file was rewritten by babel and Closure processes this thinking it is CommonJS

thheller20:03:26

so it rewrites to .default.default

thheller20:03:08

basically closure would need to detect/support the __esModule idiom for this to work

thheller20:03:16

currently they don't

thheller20:03:26

meant to open an issue about this a while ago but forgot

jannis20:03:14

That makes sense. Two questions: What makes Closure think it’s CommonJS? And is there a workaround?

thheller20:03:24

module$Users$jannis$Work$oss$groom$node_modules$symbol_observable$lib$index["default"]["default"] this is the issue

thheller20:03:55

ES6 = has import or export

thheller20:03:11

CJS I believe as soon as it finds require or exports

thheller20:03:26

workaround not that I'm aware of. well .. what I'm doing in shadow-cljs but thats really an entirely different implementation

jannis20:03:37

@thheller Do you have a good rep with the Closure team? I wouldn’t be comfortable filing an issue with my half-knowledge but I’d be happy to work on a fix.

thheller20:03:16

not really. this was the first thing I ever reported there https://github.com/google/closure-compiler/issues/2822

thheller20:03:11

the "fix" however would be pretty involved I think. goes deeply into their rewriting code and ES6/CJS interop which is already pretty tricky as it is

jannis20:03:35

Right. I wonder if there’s a better chance to change either apollo-client (which does the import Thing from "symbol-observable") or symbol-observable to prevent this…

thheller20:03:06

best option is always making the raw ES code available to closure

thheller20:03:39

so probably symbol-observable adding "module" to there package.json and adding the sources

jannis20:03:40

Hehe, that would be nice–but I can’t use "module" because it breaks requiring graphql-js (which has an .mjs file in its "module" entry and was the reason for the new :package-json-resolution option).

jannis20:03:17

Hm. symbol-observable has a "jsnext:main" entry though that points to the ES sources.

jannis20:03:49

Let me try adding that.

thheller20:03:26

closure works great if you feed it pure ES6

thheller20:03:31

webpack/babel rewritten stuff not so much

jannis20:03:49

Ok, that makes other npm deps pick up ES6 sources as well and apparently, Closure doesn’t support wildcard exports yet:

ERROR - ES6 transpilation of 'Wildcard export' is not yet implemented.
export*from"./link";
^^^^^^^^^^^^^^^^^^^^
...
ERROR: JSC_CANNOT_CONVERT_YET. ES6 transpilation of 'Wildcard export' is not yet implemented.

jannis20:03:24

(I changed :language-in to :es6)

thheller20:03:36

well in shadow-cljs I added the ability to override how to resolve things. https://shadow-cljs.github.io/docs/UsersGuide.html#_resolving_packages

thheller20:03:23

so you could add :resolve {"symbol-observable" {:target :npm :require "symbol-ovservable/path/to/es-main"}}

thheller20:03:01

nothing comparable in Core yet but it has been extremely valuable for a lot of issues in the npm world so it might be worth considering something like this for core

jannis20:03:25

Maybe. It’s a bit like a per-dependency :package-json-resolution option except a little more flexible. I wonder if it would work at all in core though, with us relying on resolving modules the same way Closure does.

thheller20:03:20

right ... its probably not so easy to add this since I also rewrite code before passing it to Closure

jannis20:03:53

… I wonder if I can somehow hook into the CLJS build process to rewrite symbol-observable/lib/index.js before it is processed. That would be so hacky but it would work. 😄

jannis20:03:59

(I know, I shouldn’t do that)

thheller20:03:07

no idea. I no longer know how any of this works outside shadow-cljs 😛

thheller20:03:37

but thats basically what I'm doing in shadow-cljs so sounds like a good plan

thheller20:03:49

process the JS before giving it to Closure I mean

jannis20:03:28

I’ll sleep on it 😉