Adds freeze option for opting-out of Object.freeze usage
#1696
Conversation
bin/src/index.js
Outdated
| @@ -17,6 +17,7 @@ const command = minimist( process.argv.slice( 2 ), { | |||
| h: 'help', | |||
| i: 'input', | |||
| l: 'legacy', | |||
| z: 'frozen', | |||
There was a problem hiding this comment.
I don't think quite specific feature should have an alias. Writing the full version will be less confusing.
bigtimebuddy
changed the title
frozen option for opting-out of Object.freeze usage
There was a problem hiding this comment.
Besides my suggestion to only make this an output option we NEED a test before this can be merged. Otherwise there is no guarantee future refinements will not break this functionality.
If you need any help, please tell us. Otherwise I think this is something useful we could put into the next feature release.
bin/src/run/mergeOptions.js
Outdated
| @@ -12,6 +12,7 @@ export default function mergeOptions ( config, command ) { | |||
| const inputOptions = { | |||
| input: getOption('input'), | |||
| legacy: getOption('legacy'), | |||
| frozen: getOption('frozen'), | |||
There was a problem hiding this comment.
As freezing is something that only influences the form of the output, I would consider this to be an output option only and not something that belongs to the general options as well, what do you think? Having this duplicity between input and output options seems to me rather confusing.
There was a problem hiding this comment.
I agree, good suggestion.
|
Definitely will add unit-test(s). Thanks for reviewing @lukastaegert. |
bigtimebuddy
changed the title
frozen option for opting-out of Object.freeze usagefreeze option for opting-out of Object.freeze usage
|
I added a unit-test for |
The test was working because there, normal and "output" options are merged. In the binary, this was not working since here, output options are only passed to bundle.generate() and therefore to Bundle.render(). With this change it should work now as intended.
|
Thanks, great test! There was just one issue left with the handling of options, see my comment. Otherwise I think this is fine now and can go into the next version. |
|
Nice work! Thanks for this |
|
I have started to assemble a branch for the next release but it will take another few days until I release it because I there are a few more PRs I want to have in there which are not ready yet. |
Addresses #1693
Added
This PR adds the config option
freeze(defaults totrue). When set tofalse, it will no longer useObject.freezefor imported namespaces (import * as foo from ‘foo-module’).Provides a convenient opt-out for namespace protection in cases where it is undesirable for projects. In the attached issue, the use-case is injecting deprecation warnings for removed namespace members.