-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Adds babel caching for system.js. #8364
Conversation
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 1 Live output at: http://107.21.233.14:8877/cd573f76509e4e0/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 1 Live output at: http://54.215.176.217:8877/955924f01f74495/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/955924f01f74495/output.txt Total script time: 21.01 mins
|
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'll test this properly after a good night's sleep, but for now just two quick comments based on skimming through the code.
*/ | ||
var babel = require('plugin-babel'); | ||
|
||
var cacheExpiration = 30 /* min */ * 60 * 1000; |
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.
Given that the runtime on the Linux bot is already >30 minutes, and that PR #8284 will make the tests slightly slower, would it perhaps make sense to use 60 minutes
instead to avoid cache invalidation during test runs?
dbPromise = new Promise(function (resolve, reject) { | ||
var request = indexedDB.open(dbName, dbVersion); | ||
request.onupgradeneeded = function() { | ||
var db = request.result; |
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.
Nit: Too much indentation here, and in the two methods just below.
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/cd573f76509e4e0/output.txt Total script time: 60.00 mins |
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/714cbbde8b8d8ae/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/1f50148b27e06a1/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/1f50148b27e06a1/output.txt Total script time: 20.82 mins
|
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/714cbbde8b8d8ae/output.txt Total script time: 32.95 mins
Image differences available at: http://107.21.233.14:8877/714cbbde8b8d8ae/reftest-analyzer.html#web=eq.log |
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've tested this successfully locally, both with the current master
and also together with PR #8284.
This works great in my testing, so thank you for implementing this!
- checks for crypto.webkitSubtle in addition to crypto.subtle - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest - https://msdn.microsoft.com/en-us/library/dn302325%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396
- checks for crypto.webkitSubtle in addition to crypto.subtle - fixes a problem where the viewer no longer loads in development mode on Safari - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest
…n Safari - the viewer no longer loads in development mode on Safari - this is due to Safari having crypto.webkitSubtle instead of crypto.subtle - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest - maintainer sentiment: people who develop on Safari can get this speedup once Safari drops prefix for SubtleCrypto.
…n Safari - the viewer was not loading in development mode on Safari, due to Safari having crypto.webkitSubtle instead of crypto.subtle. - the isCachingPossible check was amended to check for crypro.subtle which is currently not in Safari but in Firefox and Chrome. This essentially works around the issue by disabling caching for Safari in development mode. - maintainer sentiment: people who develop on Safari can get this speedup once Safari drops prefix for SubtleCrypto. - mozilla#8387 (comment) - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest
…n Safari - the viewer was not loading in development mode on Safari, due to Safari having crypto.webkitSubtle instead of crypto.subtle. - the isCachingPossible check was amended to check for crypro.subtle which is currently not in Safari but in Firefox and Chrome. This essentially works around the issue by disabling caching for Safari in development mode. - maintainer sentiment: people who develop on Safari can get this speedup once Safari drops prefix for SubtleCrypto. - note: at time of writing Safari Version 10.1 (12603.1.30.0.34) has an issue where caching can be enabled for PDF.js but must to be disabled for worker, otherwise the two sides do not communicate. - mozilla#8387 (comment) - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest
…n Safari - the viewer was not loading in development mode on Safari, due to Safari having crypto.webkitSubtle instead of crypto.subtle. - the isCachingPossible check was amended to check for crypro.subtle which is currently not in Safari but in Firefox and Chrome. This essentially works around the issue by disabling caching for Safari in development mode. - maintainer sentiment: people who develop on Safari can get this speedup once Safari drops prefix for SubtleCrypto. - note: at time of writing Safari Version 10.1 (12603.1.30.0.34) has an issue where caching can be enabled for PDF.js but must to be disabled for worker, otherwise the two sides do not communicate. - mozilla#8387 (comment) - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest
amends Babel cache (#8364) implementation to also work on Safari
Adds babel caching for system.js.
…n Safari - the viewer was not loading in development mode on Safari, due to Safari having crypto.webkitSubtle instead of crypto.subtle. - the isCachingPossible check was amended to check for crypro.subtle which is currently not in Safari but in Firefox and Chrome. This essentially works around the issue by disabling caching for Safari in development mode. - maintainer sentiment: people who develop on Safari can get this speedup once Safari drops prefix for SubtleCrypto. - note: at time of writing Safari Version 10.1 (12603.1.30.0.34) has an issue where caching can be enabled for PDF.js but must to be disabled for worker, otherwise the two sides do not communicate. - mozilla#8387 (comment) - https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/digest
…crypto amends Babel cache (mozilla#8364) implementation to also work on Safari
Makes developer viewer loading faster.