Skip to content
This repository has been archived by the owner on Oct 6, 2022. It is now read-only.

Enhance Rendertron config #267

Merged
merged 4 commits into from Mar 13, 2019
Merged

Enhance Rendertron config #267

merged 4 commits into from Mar 13, 2019

Conversation

gravi2
Copy link
Contributor

@gravi2 gravi2 commented Feb 9, 2019

This PR enhances the config options exposed/available to rendertron. It adds the following config options in addition to the existing 'datastoreCache' option

timeout: number;
port: string;
width: number;
height: number;

Respective default values are also been added:

datastoreCache: false,
timeout: 10000,
port: '3000',
width: 1000,
height: 1000,

This PR helps address following open issues:

  1. page.setViewport() to 1280x768 #213 , Pass resolution to 'render' #184 (Allow setting viewport/resolution)
  2. timeout option in rendertron middleware doesn't do anything #160, Change renderbudget timeouts #211 (Allow setting timeout)
  3. How to specify port #253 ( Specify port in config )

Also includes following Improvements:

  1. npm run monitor and monitor-inspect will now listen for any src/*.ts file changes, build the rendertron.js file and reload the server. This helps for smoother development process.
  2. Added nodemon.json config file to control nodemon options.
  3. Added documentation in README for the new config options.

@gravi2
Copy link
Contributor Author

gravi2 commented Mar 1, 2019

@AVGP: do you have any feedback on the PR or anything I can help to get the PR merged?

Copy link
Collaborator

@AVGP AVGP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @gravi2 - sorry for the delay and thanks for the contribution!
I'm really happy to see the config evolve and I think this PR is basically ready to be merged.

The one nit-pick I have is that I'm not sure why we need the additional dependency of ts-node - would you mind elaborating on that?

package.json Outdated Show resolved Hide resolved
@gravi2
Copy link
Contributor Author

gravi2 commented Mar 13, 2019

@AVGP I have removed the ts-node dependency as requested. I might have added it for something but we longer seem to need it.
Also BTW I have reverted the package-lock.json file to that of master, since I see there is existing issue in master. See my comment in the issue linked below:

#281 (comment)

@AVGP
Copy link
Collaborator

AVGP commented Mar 13, 2019

Awesome thank you, @gravi2

@AVGP AVGP merged commit 05b8469 into GoogleChrome:master Mar 13, 2019
@gravi2 gravi2 deleted the rendertron-config branch March 13, 2019 09:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants