-
Notifications
You must be signed in to change notification settings - Fork 362
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
Added a --visualize option to generate build output stats #528
Conversation
Updated readme Added the new visualize flag Added a test with a postbuild script that copies the stats.html Allow tests to have a postbuild script! :)
Hmm, I really can't get my snapshots to match travis. Running on OSX here. Does anyone have any idea what's going on? |
"name": "visualize", | ||
"scripts": { | ||
"build": "microbundle --visualize", | ||
"postbuild": "mv stats.html dist/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can remove the need for a postbuild
task. The docs of rollup-plugin-visualize
don't mention it, but you can pass a directory with the filename
setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like a good plan.
@marvinhagemeister Do you have any idea about the other (unrelated) snapshots that are failing?
Last time I checked master is also failing too... so not sure what to do next.
@AlexHayton sorry for the slow reply - I also have the issue with snapshots being out of sync on Mac OS. Unfortunately, I haven't yet found a fix for this :/ |
Sorry I've been away from this for a while. Life got in the way. I'll try and resubmit soon. I worked out why the snapshots were failing, it is an issue with the randomly generated CSS class names in the visualiser output. So I am hoping it's possible to pass a specific seed or just take those out of the snapshots... |
@developit I think found a way around this, I will submit it seperately... |
Just an update: this should all work without the Docker shennanigans now that we're on GH actions. Sorry! |
Wow, zombie thread back from the dead 😄 |
Actually reading my past self's comments, the specific blocker here was the CSS class names... Time to get the thinking caps on! |
No pressure btw! I like the idea of this feature though, and I'd be happy to rebase - I don't have the snapshots issues when working in the repo anymore (perhaps they're still there...) |
Ended up implementing in #549 rather than editing this PR directly. It's finally merged! |
Following up from #526
Updated readme
Added the new visualize flag
Added a test with a postbuild script that copies the stats.html
Allow tests to have a postbuild script! :)