Do not pollute global scope in cli

[stereobooster]

According antv/g documentation it should be possible to avoid polluting global scope:

pintora/packages/pintora-cli/src/render.ts

Lines 35 to 37 in 8ec6708

	// setup the env for renderer
	global.window = dom.window as any
	global.document = document

PS. there are faster JSDOM alternatives, I wonder if it would have any impact on the performance:

• https://github.com/WebReflection/linkedom
• https://github.com/capricorn86/happy-dom

[hikerpig]

1. You do have the point, I will try to solve this pollution issue.
2. I assume the performance bottleneck is not in dom manipulation because we do not animate the dom or so. Text calculation / layout are more likely be the case.

[hikerpig]

Sadly, those global assignment are required for all the things to work for antv g now, because it mainly targets to browser environment.
I will have to wrap all the thing and run them in a vm context to prevent polluting global scope.

[tex0l]

I had a hard time debugging why I got a JSDom error without using directly using JSDom, apparently it is because of the pollution caused by importing @pintora/cli into my project, and another package infers if they are in a browser or not by detecting if window is defined... which makes this package fail when @pintora/cli has been already executed...

Why do you need a global scope? The link @stereobooster referenced explains how to avoid using globals with '@antv/g', are you in an edge case where it does not work?

[hikerpig]

@tex0l Accually the method @antv/g's documentation decribes is not enough for it to run inside Node.js enviroment - it depends on other globals that is only available in browser.
Currently I need to pollute at least as this snippet shows.

I think you problem might have something to do with the window pollution ?

  // setup the env for renderer
  global.window = dom.window as any
  global.document = document
  ;(dom.window as any).devicePixelRatio = devicePixelRatio
  ;(global as any).CanvasPattern = CanvasPattern

I was thinking about creating a vm inside pintora/cli or adjusting the documentation about how to import @pintora/cli inside a vm. I didn't have time back then for a proper solution.

[hikerpig]

@tex0l Sorry about wasting your time on this. I will need some time to think about the proper isolation solution to fix it all.
Right now I can only think of a quick fix to restore globals as it was before pintora render method is called. Most of the time it should be OK.

[tex0l]

Don't worry, I'm sorry if my message was rude, that wasn't the intention, I just posted it so that people with a similar issue could maybe debug their issue using this insight.

I couldn't manage to use a VM (because I cannot import modules in a VM without polluting the global scope as well) nor a worker (because node canvas is not compatible with workers). Therefore I resorted to using a child_process.fork and passing the result through IPC encoded in base64 when it is a Buffer: tex0l/astro-pintora#5

EDIT: this solution does not work because of bundling issues.. back to square one.

[hikerpig]

Right now I can only think of a quick fix to restore globals as it was before pintora render method is called. Most of the time it should be OK.

@tex0l Try updating to @pintora/[email protected] and see if it gets better.

[tex0l]

Hi @hikerpig ,

Thanks for the attempt, unfortunately it is flaky when multiple render runs are started in parallel.. I've added logs in the patcher methods so that you can understand why it still fails:

patching global window
patching global document
patching global CanvasPattern
patching global window
patching global document
patching global CanvasPattern
restoring globals
restoring globals

I'll implement a waterfall on my end to ensure no parallel runs can happen.. But that's a bit disappointing in terms of performance.

[tex0l]

With both fixes combined (yours to revert the globals and mine to waterfall calls), I managed to make my bug disappear.

I wouldn't consider this issue closed, as it technically still pollutes the global object briefly, and the function being async there is no guarantee that nothing is executing while the globals are still patched.

[tex0l]

ERRATUM: my bad, I ran into the issue where an execution of render happens at the same time as my other function which needs to have an unpolluted env... I am therefore forced to expose my waterfall function to make sure this function is never ran in parallel with the render function..

This is very hacky, but it technically works.

[hikerpig]

@tex0l Yes it's not an elegant solution, let's keep this issue open and tracked.

I will do some experiments on vm and child_process approach.

[hikerpig]

I've come to a solution of running actual rendering in a forked subprocess #276.
@tex0l You may try this version @pintora/[email protected]

[hikerpig]

Merged into @pintora/[email protected]