Skip to content

Conversation

@hybrist
Copy link
Contributor

@hybrist hybrist commented Apr 9, 2021

This adds a test that optimizes the TypeScript using @tscc/tscc which combines tsickle and closure compiler with advanced optimizations. The test acts as a rough validation that the library doesn't break when properties are renamed.

Closure compiler will aggressively strip fields or rename them if it thinks they are unused or that it knows all possible use sites. The TypeScript types can already take care of most of the issues but a big exception is meta programming: Dynamic property lookups and -writes need to happen via quoted or bracketed access.

@hybrist hybrist force-pushed the advanced-optimized branch 4 times, most recently from e28d7b0 to 3109d97 Compare April 9, 2021 16:13
'guessed boolean negation')

// Default values for types:
deepEqual(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This assertion fails before #371.

@hybrist hybrist force-pushed the advanced-optimized branch from f4830db to 5e46c91 Compare April 9, 2021 16:18
@@ -0,0 +1 @@
../../lib No newline at end of file
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed to add a symlink so that we both get a fresh project root and not have generated closure namespaces with .. in them.

"name": "optimized-test",
"version": "0.0.0",
"dependencies": {
"@tscc/tscc": "^0.6.4"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The bad news here: It's using a slightly different version of TypeScript (3.9?) right now.

@hybrist hybrist force-pushed the advanced-optimized branch from 5e46c91 to 394d64e Compare April 9, 2021 16:23
@hybrist hybrist force-pushed the advanced-optimized branch from 394d64e to 1cb035b Compare April 9, 2021 16:24
@hybrist hybrist marked this pull request as ready for review April 10, 2021 05:01
@bcoe
Copy link
Member

bcoe commented Apr 11, 2021

@jkrems awesome, thank you for adding this test 👍

@hybrist
Copy link
Contributor Author

hybrist commented May 30, 2021

Would you want to combine the two PRs into one? Or is there anything missing on my end to enable landing this?

@bcoe bcoe changed the title Add test for optimized output test: add test for optimized output Jun 20, 2021
@bcoe bcoe merged commit 6877a2d into yargs:master Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants