Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass custom arguments to puppeteer #25

Closed
floriantraber opened this issue Nov 21, 2017 · 5 comments
Closed

Pass custom arguments to puppeteer #25

floriantraber opened this issue Nov 21, 2017 · 5 comments
Assignees
Milestone

Comments

@floriantraber
Copy link

Certain setups may make it necessary to launch puppeteer with different options (e.g. sandbox). Convert-Svg should have some option to pass arguments to puppeteer.

Relevant code:
https://github.com/NotNinja/convert-svg/blob/dc7ad91eb54565217ceea862d1085cad8c14b040/packages/convert-svg-core/src/Converter.js#L269

@neocotic
Copy link
Owner

@floriantraber I definitely want to add this. I was going to do it initially, but I really wanted to get it stable first. If we go ahead with this, there should be some caveats;

  • puppeteer launch options can only be passed to the main API methods (incl. createConverter)
  • puppeteer launch options cannot be passed to the instance methods on the Converter instance returned by API#createConverter

This is because each Converter instance holds a reference to the browser (once created for the first time) for reuse in order to optimise performance. I don't see this as an issue, but I just wanted to highlight it.

This would be especially good for those wanting to specify their own executable path, which is why I first wanted to implement this.

For simplicity, I'm thinking that we simply accept these options as a puppeteer field on the exist options parameters for API#convert and API#convertFile and we add a new optional options parameter to API#createConverter that accepts the same.

@neocotic
Copy link
Owner

Also, I'm not planning on making these options part of the CLI for now. This will be more complicated and I might create a separate issue to track that, which may be part of a future release.

@neocotic
Copy link
Owner

I have implemented this under PR #27 and it will be included in the 0.3.2 release.

@neocotic neocotic mentioned this issue Nov 21, 2017
3 tasks
@neocotic
Copy link
Owner

@floriantraber I've just released v0.3.2 which should include this feature. Please play around with it and let me know how you get on. I wasn't able to fully test it due to the nature of it, so I'm hoping you can let me know how you get on.

@floriantraber
Copy link
Author

works flawlessly, thanks

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

No branches or pull requests

2 participants