ts-transformer-properties-rename icon indicating copy to clipboard operation
ts-transformer-properties-rename copied to clipboard

Add errors/warning where we're sure that an error could be

Open timocov opened this issue 4 years ago • 7 comments

Related to https://github.com/timocov/ts-transformer-properties-rename/issues/8#issuecomment-651124733

timocov avatar Jun 29 '20 13:06 timocov

Needs to revert changes for #5 and warn about using in operator for internal types (that's ok for public types).

timocov avatar Jun 30 '20 08:06 timocov

Depends on https://github.com/microsoft/TypeScript/issues/36733

timocov avatar Jun 30 '20 10:06 timocov

~Quite possible that we need to revert changes from dc111965e67ad6d4f63117e176c31e1862cba721 and warn about defining properties because at least terser doesn't support properties renaming defined in Object.defineProperty. From the docs https://terser.org/docs/cli-usage#cli-mangling-property-names-mangle-props:~

~Avoid calling functions like defineProperty or hasOwnProperty, because they refer to object properties using strings and will break your code if you don't know what you are doing.~

~So I guess we need to check whether a compilation require to use defineProperty to define properties and whether this property is internal and then warn about that.~

Terser supports mangling properties defined in defineProperty so it's irrelevant.

/cc @artfiedler

timocov avatar Jul 04 '20 10:07 timocov

Nooooooo….. Lol I believe those docs are out of date, I had verified terser worked, I had the latest version of terser

From: Evgeniy Timokhov Sent: Saturday, July 4, 2020 5:21 AM To: timocov/ts-transformer-properties-rename Cc: Arthur Fiedler; Mention Subject: Re: [timocov/ts-transformer-properties-rename] Add errors/warningwhere we're sure that an error could be (#10)

Quite possible that we need to revert changes from dc11196 and warn about defining properties because at least terser doesn't support properties renaming defined in Object.defineProperty. From the docs https://terser.org/docs/cli-usage#cli-mangling-property-names-mangle-props: Avoid calling functions like defineProperty or hasOwnProperty, because they refer to object properties using strings and will break your code if you don't know what you are doing. /cc @artfiedler — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

artfiedler avatar Jul 04 '20 10:07 artfiedler

I believe those docs are out of date, I had verified terser worked, I had the latest version of terser

Yep, I've tested it after posting my comment and it works fine.

timocov avatar Jul 04 '20 11:07 timocov

Related to https://github.com/terser/terser/issues/748

timocov avatar Jul 04 '20 11:07 timocov

Avoid calling functions like defineProperty or hasOwnProperty, because they refer to object properties using strings and will break your code if you don't know what you are doing.

This is merely to warn people who don't understand the limitations. I got tons of issues which assumed Terser did full flow analysis (which it doesn't).

Ideally I would love property mangle to work more safely, and using typescript typing information to achieve this is a great idea!

fabiosantoscode avatar Jul 05 '20 22:07 fabiosantoscode