♻️ Extract type assertions out of log to multiple pure function calls #34088
Conversation
|
Hey @jridgewell! These files were changed: |
|
This pull request introduces 1 alert and fixes 1 when merging eaaa9e7 into e7e5b4e - view on LGTM.com new alerts:
fixed alerts:
|
|
@jridgewell bump |
|
This pull request introduces 1 alert and fixes 1 when merging f2b454d into 6a9ada0 - view on LGTM.com new alerts:
fixed alerts:
|
| */ | ||
| export function assertElement(sentinel, shouldBeElement, opt_message) { | ||
| return assertions.assertElement( | ||
| /** @type {!AssertionFunction} */ (assert), |
There was a problem hiding this comment.
I'm concerned that none of these are going to be properly DCE'd. The indirection here requires Closure to understand that assert is empty, so assertElement's call to assert is empty, so assertElement is empty, which is difficult (and likely impossible to do if we switch to Terser).
Instead, if the intention is for dev.* to never throw, explicitly check for isMinifiedMode is each.
There was a problem hiding this comment.
Looked at dist output and you were right. Dropped the ifMinifiedMode bailout into all the dev helpers; looks like they're DCEing correctly based on some manual tests (they aren't used anywhere yet so they won't show up in bundle-size bot or v0.js output)
|
This pull request introduces 7 alerts and fixes 1 when merging 74ade7c into 1f4187e - view on LGTM.com new alerts:
fixed alerts:
|
…ampproject#34088) * Make assert its own directory * Fix type annotation on isString * Update log * Move type assertion logic out of log * Provide user/dev assertion modules * Take assertFn as parameter * Add comments * Support Array args to assertion functions * Missing things * Remove commented-out import * Remove outdated comment * Remove sentinel param from dev/user assert files * DCE escape hatch on dev assert helpers * Add typecasts for DCE bail-outs * Update forbidden terms allowlist
Moves the logic into
src/core/assert.src/logcalls the base implementations, which wrap the message extraction logic (which can't yet migrate into core)