uber-web
Conditional import of `asciify-image` is not compatible with vite.js
The code added for #34 is not compatible with vite.js:
https://github.com/uber-web/probe.gl/blob/7734e0158c9af9d6debcd6c7ed1869c21654a40b/modules/log/src/log.ts#L426-L442
The problem is that vite will move all imports to the top of the file, so it's moved outside of the try/catch block. While this is probably also a bug in vite.js, it's also a bit questionable to depend on the behaviour of other bundlers.
We require a workaround described in https://github.com/visgl/deck.gl/issues/6134#issuecomment-1201150859.
I think we're running into the same issue. I can see that #197 tries to resolve this, but I can't find the new version on npm yet. Any idea when it'll be published?
Hey damn, that was quick. Thank you @ibgreen! It does seem like the server engine I'm using does not honor the browser section in the package.json - or if it does it still messes up somewhere due to the try catch. I've opened up an issue with the maintainers of the engine as well, very interested to see what they think as it does seem quite uncommon. Just out of curiosity and if you dont mind: Do you feel like being able to print ascii images in a pure node env. is worth all the workarounds and complications it brings with it? I never really came into contact with proble.gl as it's just a dep. of deck.gl for me, so I would be curious on the usage.
Do you feel like being able to print ascii images in a pure node env. is worth all the workarounds and complications it brings with it?
No, if it turns out to be a very hard problem we could absolutely drop that bit. But deck.gl uses loaders.gl extensively, and loaders.gl is heavily dependent on browser field to handle different implementations and dependencies in browser and node, so if this doesn't work I would be nice to understand why this fails but loaders.gl does not, before we drop it.
Hi @ibgreen @warflash not sure if my issue is related to this one, but sounds very similar, since very recently my vite build is failing because 'nodeAsciifyImage' is not exported. I'm also using Deck.gl which depends on [email protected].
No telling when Deck.gl will update their dependencies so I'm currently trying to override to [email protected] in my package.json "resolutions".
Does this sound like the same issue you've addressed in 3.5.1 @ibgreen ?
error during build:
Error: 'nodeAsciifyImage' is not exported by __vite-browser-external, imported by node_modules/@probe.gl/log/dist/esm/log.js
at error (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:198:30)
at Module.error (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:12543:16)
at Module.traceVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:12902:29)
at ModuleScope.findVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:11554:39)
at FunctionScope.findVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:6486:38)
at ChildScope.findVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:6486:38)
at Identifier.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:7553:40)
at CallExpression.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:5383:23)
at CallExpression.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:9049:15)
at ExpressionStatement.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:5383:23)
error Command failed with exit code 1.
Hi @NicBathgate I meet the same problem "build fault because 'nodeAsciifyImage' is not exported." I already update the probe.gl to 3.5.1 but nothing change . Could you give me an idea , thanks!
Hi @ibgreen @warflash not sure if my issue is related to this one, but sounds very similar, since very recently my vite build is failing because 'nodeAsciifyImage' is not exported. I'm also using Deck.gl which depends on [email protected].
No telling when Deck.gl will update their dependencies so I'm currently trying to override to [email protected] in my package.json "resolutions".
Does this sound like the same issue you've addressed in 3.5.1 @ibgreen ?
error during build: Error: 'nodeAsciifyImage' is not exported by __vite-browser-external, imported by node_modules/@probe.gl/log/dist/esm/log.js at error (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:198:30) at Module.error (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:12543:16) at Module.traceVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:12902:29) at ModuleScope.findVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:11554:39) at FunctionScope.findVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:6486:38) at ChildScope.findVariable (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:6486:38) at Identifier.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:7553:40) at CallExpression.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:5383:23) at CallExpression.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:9049:15) at ExpressionStatement.bind (/home/runner/work/u1-client-web/u1-client-web/node_modules/rollup/dist/shared/rollup.js:5383:23) error Command failed with exit code 1.
@cocaine-coder I'm still getting the same build failure due to 'nodeAsciifyImage', haven't been able to release important production changes to our product because of this issue and the workaround posted here does not seem to work for me :(
@ibgreen is it possible to roll another release with the 'nodeAsciifyImage' change reverted or the issue worked around in a different way? It's not possible for us to remove deck.gl from our project, so my only other option is to rip Vite out and use a different build tool, which I really don't want to do :(
@cocaine-coder I'm still getting the same build failure due to 'nodeAsciifyImage', haven't been able to release important production changes to our product because of this issue and the workaround posted here does not seem to work for me :(
Just to clarify: https://github.com/visgl/deck.gl/issues/6134#issuecomment-1201150859 is what we needed to support deck.gl (which has the probe.gl dependency) with vite prior to #197. I did not find time to check wether #197 would work for our webapp - so I can't comment on the current situation.
However, by the comments here, it suggests that deck.gl was working with vite without hacks like my workaround for you (@NicBathgate and potentially @cocaine-coder), before #197?
We couldn't make deck.gl work in vite without the mentioned workarounds from https://github.com/visgl/deck.gl/issues/6134#issuecomment-1201150859 (namely @originjs/vite-plugin-commonjs, our custom stubPlugin and define: { process.env[...] })
If it worked for you: How? What's your setup: vite version / deck.gl version / config? Maybe it's enough to document how to make deck.gl work with vite.
@ibgreen is it possible to roll another release with the 'nodeAsciifyImage' change reverted or the issue worked around in a different way? It's not possible for us to remove deck.gl from our project, so my only other option is to rip Vite out and use a different build tool, which I really don't want to do :(
If deck.gl / probe.gl worked with vite in the recent past, without my workarounds or hacks, it would imply that no change was necessary = this issue would be bogus and the quarantine wasn't needed - and then it should be reverted probably.
One thing I noticed with PR #197 on a quick review (without knowing how the browsers field behaves), was also that it didn't seem to have any nodeAsciifyImage function for logImageInNode and there's no conditional check if that function is defined either, before trying to use it - I assume that's where the current error stems from.
@JannikGM thanks for your response! This https://github.com/visgl/deck.gl/issues/6134#issuecomment-1201150859 workaround did not work for me with #197 , and I haven't been able to test that workaround prior to #197 since it's been merged and released already.
My vite build with deck.gl (and by extension probe.gl) was working perfectly up until August 10th, and the next built I tried on August 12th failed with the 'nodeAsciifyImage' is not exported by __vite-browser-external issue.
Considering 3.5.1 including #197 was released on August 11th I have a strong suspicion that #197 is actually what is causing this vite build issue. This is why I suggest reverting #197 . @ibgreen what are your thoughts, is a revert possible?
Yes, #197 is definitely causing the problem you describe, probably because there's a bug in #197.
However, deck.gl never worked with vite (before #197) for us either, unless we had the workarounds shown in https://github.com/visgl/deck.gl/issues/6134#issuecomment-1201150859. So I'm curious as to how you even used deck.gl with vite before #197 if you didn't use some workarounds in your vite config. #197 was a step towards reducing the number of workarounds (required before #197, described in https://github.com/visgl/deck.gl/issues/6134#issuecomment-1201150859). Ideally deck.gl+vite would work out-of-box, but it didn't for us. Hence #197 trying to improve the situation (which ended up introducing a new problem).
Hence (summary of my previous post): If deck.gl+vite worked before August 11th for you, how did you set up your vite that you didn't need the workaround? What version of deck.gl were you using etc.? If my workaround wasn't needed in the first place, then #197 wasn't necessary and should be reverted. If you also required workarounds before, then #197 should stay, and be improved.
Fyi you guys that have failing builds with the new patch can use npm/yarn dep overwrites/resolutions to pin the Version to 3.5.0. Maybe the workaround did not work for me not due to the bundler not respecting the Browser field but because of the same issues you are facing. I have a big suspicion theres better ways out there that dont introduce a bunch of Potential Bugs for conditional dependency Imports but I cant think of any of the top of my head. Easiest for now I guess would be dropping asciifyimage until theres a way to nicely handle the import
I don't have problem bundling 3.5.0 or 3.5.1 using esbuild directly. When using vite, app builds successfully with 3.5.0, but fails with 3.5.1. So this is consistent with everyone else's report in this thread. I'm not sure what exactly caused @JannikGM 's issue to begin with, maybe some other custom configuration? Do you mind sharing your tsconfig file?
@ibgreen The error from 3.5.1 ('nodeAsciifyImage' is not exported) is the result of trying to import a named function from null. I'm opening a PR to fix this.
Took your advice thanks @warflash , for anyone else wondering exactly how to do this, this worked for me in package.json (we use yarn):
"resolutions": {
"*/**/probe.gl": "3.5.0",
"*/**/@probe.gl/log": "3.5.0"
},
Slightly ironic in that we published the 3.5.1 patch because people were having problems bundling probe.gl 3.5.0 with vite, and now the solution to bundling probe.gl with vite is to use resolutions to pin probe.gl to 3.5.0 ...
Anyway, 3.5.2 is out now, improving the fix in 3.5.1.