Clojurians Log v2

I removed the automatic polyfill for node built-in packages. still need to figure out a better automatic handling, until then npm install process

Just encountered a similar error, I think. Given the most recent commit that brings back some polyfills, maybe it makes sense to just include all of them? My error in particular is about the fs package when building antlr4@4.8.0.

no I removed the automatic polyfilling on purpose

Hmm, and I still can't run the build even after npm i fs:

package in /home/p-himik/[...]/fs specified entries but they were all missing

webpack removed it 5 years ago. it is time. packages are not supposed to rely on this anymore. you can still all do it manually. just doing it for process because its so common

But if someone uses React with Webpack, they still don't have to add polyfills manually, or do they?

I don't really care. I wanted to get it of it and I did. I only added process back because I need to make the detection stuff I have smarter

} else if (
                "object" === typeof process &&
                "function" === typeof process.emit
              ) {
                process.emit("uncaughtException", error);
                return;
              }

this is the bit that makes my detection think process is required. it isn't really ...

I'm just perplexed because it makes shadow-cljs 3.x break for apparently no reason other than "webpack did it", without enabling the same convenience that webpack offers (I assume). So shadow-cljs 3 users have to do more work than webpack users and than shadow-cljs 2 users.

that is nonsense

just build your antlr4 with webpack5+ and get back to me

if it works without manual extra config I'll think about adjusting what shadow-cljs does

everything that was previously done is still available. just requires manual config

https://github.com/webpack/node-libs-browser

please note the "archived" status and deprecated note.

Ah, OK, so apparently it doesn't work in Webpack: https://stackoverflow.com/a/41622685/564509 Is there then an analog to fs: "empty", so I don't have to install a random polyfill?

node-libs-browser installs a bunch of nonsense packages that 99% of shadow-cljs users likely will never need. process is the exception because of react, otherwise that wouldn't be back either.

:js-options {:resolve {"fs" false}}

and again one reason that shows why still having the polyfills is bad 😛 there are better options that don't add nonsense polyfills

That worked, thanks!

As an example, this is a function from node_modules/abcjs/src/write/interactive/selection.js:

function getMousePosition(self, ev) {
	// if the user clicked exactly on an element that we're interested in, then we already have the answer.
	// This is more reliable than the calculations because firefox returns different coords for offsetX, offsetY
	var x;
	var y;
	var box;
	var clickedOn = findElementInHistory(self.selectables, getTarget(ev.target));
	if (clickedOn >= 0) {
		// There was a direct hit on an element.
		box = getBestMatchCoordinates(self.selectables[clickedOn].svgEl.getBBox(), ev, self.scale);
		x = box[0];
		y = box[1];
		//console.log("clicked on", clickedOn, x, y, self.selectables[clickedOn].svgEl.getBBox(), ev.target.getBBox());
	} else {
		// See if they clicked close to an element.
		box = getCoord(ev);
		x = box[0];
		y = box[1];
		clickedOn = findElementByCoord(self, x, y);
		//console.log("clicked near", clickedOn, x, y, printEl(ev.target));
	}
	return { x: x, y: y, clickedOn: clickedOn };
}

function getMousePosition(self, ev) {
    var clickedOn;
    a: {
      var y = self.selectables;
      if (clickedOn = getTarget(ev.target)) {
        for (var i = 0; i < y.length; i++) {
          if (clickedOn.dataset.index === y[i].svgEl.dataset.index) {
            clickedOn = i;
            break a;
          }
        }
      }
      clickedOn = -1;
    }
    if (0 <= clickedOn) {
      y = self.selectables[clickedOn].svgEl.getBBox(), y = y.x <= ev.offsetX && y.x + y.width >= ev.offsetX && y.y <= ev.offsetY && y.y + y.height >= ev.offsetY ? [ev.offsetX, ev.offsetY] : 3 > Math.abs(ev.layerY / self.scale - ev.offsetY) ? [ev.offsetX, ev.offsetY] : [ev.layerX, ev.layerY], ev = y[0], y = y[1];
    } else {
      clickedOn = y = 1;
      var svg = ev.target.closest("svg");
      i = 0;
      svg && svg.viewBox && svg.viewBox.baseVal && (0 !== svg.viewBox.baseVal.width && (y = svg.viewBox.baseVal.width / svg.clientWidth), 0 !== svg.viewBox.baseVal.height && (clickedOn = svg.viewBox.baseVal.height / svg.clientHeight), i = svg.viewBox.baseVal.y);
      ev.target && "svg" === ev.target.tagName ? (svg = ev.offsetX, ev = ev.offsetY) : (svg = ev.layerX, ev = ev.layerY);
      y = [svg * y, ev * clickedOn + i];
      ev = y[0];
      y = y[1];
      clickedOn = ev;
      i = y;
      svg = 9999999;
      for (var closestIndex = -1, i$jscomp$0 = 0; i$jscomp$0 < self.selectables.length && 0 < svg; i$jscomp$0++) {
        var el = self.selectables[i$jscomp$0];
        self.getDim(el);
        if (el.dim.left < clickedOn && el.dim.right > clickedOn &&  < i && el.dim.bottom > i) {
          closestIndex = i$jscomp$0, svg = 0;
        } else if ( < i && el.dim.bottom > i) {
          var horiz = Math.min(Math.abs(el.dim.left - clickedOn), Math.abs(el.dim.right - clickedOn));
          horiz < svg && (svg = horiz, closestIndex = i$jscomp$0);
        } else {
          el.dim.left < clickedOn && el.dim.right > clickedOn ? (horiz = Math.min(Math.abs( - i), Math.abs(el.dim.bottom - i)), horiz < svg && (svg = horiz, closestIndex = i$jscomp$0)) : (horiz = Math.abs(clickedOn - el.dim.left) > Math.abs(clickedOn - el.dim.right) ? Math.abs(clickedOn - el.dim.right) : Math.abs(clickedOn - el.dim.left), el = Math.abs(i - ) > Math.abs(i - el.dim.bottom) ? Math.abs(i - el.dim.bottom) : Math.abs(i - ), horiz = Math.sqrt(horiz * horiz + 
          el * el), horiz < svg && (svg = horiz, closestIndex = i$jscomp$0));
        }
      }
      clickedOn = 0 <= closestIndex && 12 >= svg ? closestIndex : -1;
    }
    return {x:ev, y, clickedOn};
  }

not really no. its just :simple optimizations. so guess its doing a lot inlining

Huh. But it's a dev build - why was :simple used instead of :none?

because :none is not a valid closure compiler option. :whitespace caused issues in the past, so :simple was really the only option

and simple is smart enough to filter out nonsense like this react pattern

if (process.env.NODE_ENV === 'production') {
  module.exports = require('./cjs/react.production.js');
} else {
  module.exports = require('./cjs/react.development.js');
}

not that it matters much during dev in this case but in some packages it did

And there's no way to fine-tune :simple to tell GCC to avoid inlining and variable reuse, is there?

:js-options {:variable-renaming :off :property-renaming :off} should work

inlining I don't think so. if so its not currently exposed as a mapping from the config, but it would be easy to add if you find one https://github.com/thheller/shadow-cljs/blob/eb7613c0368d41dbc173852f69d14ac73876caff/src/main/shadow/build/closure.clj#L205

Thanks!

Hmm, it seems that it's easier to just put a bunch of print statements in this case. I tried doing this:

:resolve        {"fs"                      false
                 "./interactive/selection" {:target :file
                                            :file   "src/selection.js"}}

Ah, never mind - the original file had relative imports that I have to update.

"fs/interactive/selection" would work. relative only works in context of what it is relative to

That brings up another issue - changing the copied JS file does trigger a build, but the build uses the old version of the file. The current content starts with

var spacing = require('abcjs/src/write/helpers/spacing');

The required JS dependency "abcjs/helpers/spacing" is not available, it was required by "src/selection.js".

I'm confused. are you asking about something a library is doing internally?

> changing the copied JS file does trigger a build

what does that mean? who copied what where?

Yes, the fs bit came from a different thread - completely irrelevant here. I'm trying to debug abcjs, and currently it seems that the easiest way to do it is to fill the relevant functions with console.log statements. All those functions are in node_modules/abcjs/src/write/interactive/selection.js. That file is required in abcjs only once, as var setupSelection = require('./interactive/selection');. So I guess that adding a :resolve entry for "./interactive/selection" should work. And indeed it does - I copied the original file to src in my project and all I had to do was to adjust the require statements in there. The "another issue" that I mentioned is that while changing that copied src/selection.js does trigger the build, the built itself uses the old content of that file - the file is not re-read.

ah, so the :resolve above actually does something? that surprises me 😛

and no, this isn't a supported use case

Huh? It's documented. :) https://shadow-cljs.github.io/docs/UsersGuide.html#js-resolve-npm

consider :file something that has literally never been used or tested 😛 let alone for replacing some internal npm package file

if you want to debug a npm package do it with their build setup and tooling. I don't consider that something shadow-cljs is suitable for

The thing is, I cannot reproduce the issue on their setup and tooling. I can only reproduce it in my CLJS project. > consider :file something that has literally never been used or tested 😛 Well, it works. Apart from hot reload. > let alone for replacing some internal npm package file Assuming it works exactly as it is documented to work - for NPM packages and not internal files - and it also doesn't get hot reloaded on changes to the specified .js file, would you say that fixing that part makes sense? If so, I would assume that it would also automatically fix my minor issue, even if it's unsupported.

there is no hot-reload for npm packages, partial or otherwise. this is still a commonjs file, so it will not work with hot-reload. so that is working as intended I guess 😛

Oh! A correction - I'm not talking about hot reload in the sense "the web page should automatically pick up the changes". I remember now that we've discussed it a few years ago, when I had to make some JS files a part of the same CLJS project. But in this case the problem is with using the actual JS file content during the build - hot code reload notwithstanding. So I can't even fix it by refreshing the web page, as I could with other JS files. I have to restart the whole watch process.

I do consider :resolve {"./something/relative" ...} working a bug. that should not work, so if anything I'd fix that first

Please don't! :D

relying on that is crazy dangerous and problematic

I am relying on it solely for debugging purposes though.

it replaces for everything. so if any other package also happens to have that exact relative require it would also replace that

unlikely I know, but still

why not just change the original file? why jump through this hoop?

Right in node_modules? I guess it makes sense now that I have to restart the build process anyway. But if I didn't have to restart it, I would definitely prefer to use :resolve, regardless of the potential danger.

you could use :js-package-dirs ["mine" "node_modules"]. then have mine/abcjs with the modifications and the original in node_modules

Oh, alright, that's definitely better than the hassle with changing the original file. Although I assume I would still have to restart the whole build process on any change.

touching mine/abcjs/package.json should be enough. shadow still watches that to detect installs of new versions

Checking...

node_modules/abcjs/packages.json as ewll of course

Why is the latter required? I thought that abcjs would fully come from "mine" with that config.

> you could use :js-package-dirs ["mine" "node_modules"]. then have mine/abcjs with the modifications and the original in node_modules Did not work. The modifications did not get picked up, so I assume the original version was used.

And neither of these commands triggered a build:

touch mine/abcjs/package.json
touch node_modules/abcjs/package.json

My bad - I put :js-package-dirs outside of :js-options. The :js- prefix confused me.

The changes are picked up but touch still doesn't trigger a build.

works fine for me

ah wait nvm. just tried for normal files

yeah the shadow-cljs.edn watcher is missing a try/catch I guess

metadata of vars only exists in the compiler side, so clj. it does not exist in JS, so nothing to elide

so, its as if :elide-meta is always on, even for dev builds I guess

only exception is using actual #'foo vars in your code. in that case nothing is elided at all

ok great, that's what I wanted! Thanks for confirming.