Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why does mobx force a production build in browser mode? #2296

Closed
areilly-aops opened this issue Feb 22, 2020 · 5 comments
Closed

Why does mobx force a production build in browser mode? #2296

areilly-aops opened this issue Feb 22, 2020 · 5 comments
Labels

Comments

@areilly-aops
Copy link

areilly-aops commented Feb 22, 2020

Hello! We came across an issue today with a development build providing obfuscated errors, and I'm hoping someone can provide some clarity as to why this is happening. We did confirm that the environment variable is appropriately set to "development", and our other packages (react, et. al.) are building correctly in development mode.
image

We've actually tracked the issue to this entry in package.json, which is forcing mobx to always use the minified file in browser mode:

"browser": {
    "./lib/mobx.js": "./lib/mobx.min.js",
    "./lib/mobx.module.js": "./lib/mobx.module.js"
  },

Which renders these lines in the entry point moot, as it replaces the mobx.js in the else block with mobx.min.js:

if (typeof process !== 'undefined' && process.env.NODE_ENV === 'production') {
    module.exports = require('./mobx.min.js');
} else {
    module.exports = require('./mobx.js');
}

Can anyone help?

@danielkcz
Copy link
Contributor

danielkcz commented Feb 22, 2020

I can't say why is there such a configuration, but generally, it's better if you can reproduce the issue in development. I can see it's originating from UserModel.js and you are trying to delete something in there. Another hint is the name of the method checkIfStateModificationsAreAllowed which is declared here and you can see full messages there.

export function checkIfStateModificationsAreAllowed(atom: IAtom) {

@areilly-aops
Copy link
Author

Thanks for your response. I understand why this particular error was thrown - I was mutating the state outside an action while enforcing actions.

My concern is that this is a development server, and because it's forcing the .min version in browser mode, I'm getting obfuscated errors. This took me a frustratingly long time to track down because while I'm familiar with react, I've only used redux for state management before, so I'm not yet familiar enough with mobx to be able to look at this cryptic error and immediately see what is happening. It's especially frustrating because the error thrown by the development build makes it crystal clear what is happening.

We're introducing react and mobx to a large legacy codebase with quite a few devs who are unfamiliar with both react and this style of state management. I am looking for the rationale behind this so I can figure out if I can solve this with a different config, if this is a bug that I can submit a PR for, or if this is something we're going to have to live with - which may change the calculus on which state management package we end up going with. Building in browser mode is necessary because other packages require it to build correctly for the browser.

@danielkcz
Copy link
Contributor

Ah, now I see what you mean and turns out it was actually me who did that change. I simply did not realize that field is used in development too. This is often confusing as there is no official specification on most of these fields and it depends on bundler which one it will use.

In mobx-react we don't even use browser field so I thinking if it would be too bad to remove it here as well. Webpack will definitely fallback to module field which is a better option anyway. And other bundles who don't understand ESM would go for main field which allows picking dev/prod build based on env. Since we don't have NodeJS specific environment code, it should work just fine.

@mrtnbroder I see you were original author of that browser field. Do you have some more info why is it useful to keep it around?

@radovansurlak
Copy link

radovansurlak commented Apr 2, 2020

I am having the same issue as well (getting obfuscated errors in development environment with NODE_ENV set to "development")

@mweststrate
Copy link
Member

I think we can simply drop that field. IIRC it is mostly used in case your library has some different behavior in node vs browsers, but that is something we don't do. PR welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants