mobx-state-tree icon indicating copy to clipboard operation
mobx-state-tree copied to clipboard

Enable type checking for production by default

Open jaythomas opened this issue 5 years ago • 4 comments

Feature request

Is your feature request related to a problem? Please describe. I am able to get error messages in production by applying the following to my site's webpack config:

plugins: [
  new webpack.DefinePlugin({
    // Global mobx-state-tree configuration.
    // Force type checking in production for easier debugging:
    // https://github.com/mobxjs/mobx-state-tree/pull/1337
    'process.env.ENABLE_TYPE_CHECK': '"true"'
  })
]

However if I didn't know webpack well enough I would have tried putting process.env.ENABLE_TYPE_CHECK = "true" in my javascript as the README suggests then given up in frustration. Another issue I ran into is I first tried 'process.env.ENABLE_TYPE_CHECK': "true" but then realized MST is doing a strict equality check for the literal string 'true' so wrapping the string in extra quotes is necessary for the webpack DefinePlugin to insert a string.

Describe the solution you'd like Not having clear error messages in production is a non-starter for any web application I've built in no matter the industry. If error messages are unwanted it is the implementer's job to catch and handle appropriately.

Describe alternatives you've considered

  • There could be a runtime function you could invoke to toggle type checking versus what you have now which is static environment variable check that adds further node dependency and disallows you from toggling type checking in production if you have corrupt customer data you need to debug. This situation has happened with us as we are integrating MST into our production code.
  • The environment check could be made more relaxed to just expect a truthy value rather than a literal string value.
  • The README could be fleshed out with more examples.

Additional context N/A

Are you willing to (attempt) a PR?

  • [x] Yes
  • [ ] No

jaythomas avatar Feb 18 '20 01:02 jaythomas

Feel free to PR all three changes. Mind the "false" case 😊

Op di 18 feb. 2020 01:51 schreef Jay Thomas [email protected]:

Feature request

Is your feature request related to a problem? Please describe. I am able to get error messages in production by applying the following to my site's webpack config:

plugins: [ new webpack.DefinePlugin({ // Global mobx-state-tree configuration. // Force type checking in production for easier debugging: // https://github.com/mobxjs/mobx-state-tree/pull/1337 'process.env.ENABLE_TYPE_CHECK': '"true"' }) ]

However if I didn't know webpack well enough I would have tried putting process.env.ENABLE_TYPE_CHECK = "true" in my javascript as the README suggests then given up in frustration. Another issue I ran into is I first tried 'process.env.ENABLE_TYPE_CHECK': "true" but then realized MST is doing a strict equality check for the literal string 'true' so wrapping the string in extra quotes is necessary for the webpack DefinePlugin to insert a string.

Describe the solution you'd like Not having clear error messages in production is a non-starter for any web application I've built in no matter the industry. If error messages are unwanted it is the implementer's job to catch and handle appropriately.

Describe alternatives you've considered

  • There could be a runtime function you could invoke to toggle type checking versus what you have now which is static environment variable check that adds further node dependency and disallows you from toggling type checking in production if you have corrupt customer data you need to debug. This situation has happened with us as we are integrating MST into our production code.
  • The environment check could be made more relaxed to just expect a truthy value rather than a literal string value.
  • The README could be fleshed out with more examples.

Additional context N/A

Are you willing to (attempt) a PR?

  • Yes
  • No

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-state-tree/issues/1469?email_source=notifications&email_token=AAN4NBB42PXAZCT3MPR4JMLRDM5IRA5CNFSM4KW3J4K2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IOFSNHQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBE2SEQ7OCXJGRRKDCLRDM5IRANCNFSM4KW3J4KQ .

mweststrate avatar Feb 18 '20 09:02 mweststrate

Hey @jaythomas - just checking in here. I know it's been literal years since this issue in MobX-State-Tree, but I'm curious if you'd like to submit a PR.

I'm going to label this as open to help for now. If you're still interested in workin' on it all these years later, let me know. Otherwise, we'll see who else is up to grab it.

Thanks!

coolsoftwaretyler avatar Jun 30 '23 01:06 coolsoftwaretyler

@coolsoftwaretyler unfortunately a few days after opening this issue I stopped using mobx and mobx-state-tree. I ran into compatibility problem where mobx and Vue's reactivity systems' incompatible implementations of an array was breaking reactivity in my app altogether.

It looks like the line of confusion I mentioned in the README is gone now so at least that point is resolved. A runtime function to enable type check would certainly be the most useful way to resolve this feature request in my opinion, but I don't have a stake in the issue any longer. Feel free to close this or leave open for others as you wish.

jaythomas avatar Jun 30 '23 01:06 jaythomas

Thanks @jaythomas! I'll be keeping it open I think. Appreciate you hopping back in here to reply. Sorry MST wasn't the right fit for ya at the time.

Have a good one!

coolsoftwaretyler avatar Jun 30 '23 02:06 coolsoftwaretyler