mobx: browser breakage in mobx@6 esm: "process is not defined"

Intended outcome:
i tried to upgrade from mobx@5 to mobx@6 in my esm app

Actual outcome:
browser throws error in mobx.esm.js upon encountering reference to node-specific process.env

How to reproduce the issue:

  1. in this codepen example, open your browser’s js console, run the codepen, and you’ll see the error process is not defined
  2. now switch to mobx@5 by replacing the import map:
    {
      "imports": {
        "mobx/": "https://unpkg.com/mobx@5.15.7/",
        "mobx": "https://unpkg.com/mobx@5.15.7/lib/mobx.module.js"
      }
    }
    
  3. run again and you’ll see the problem is gone in v5, and the mobx module is logged to console

My thoughts:

  • the problem appears when i move from @5.15.7 mobx.module.js to @6.0.1 mobx.esm.js
  • i see process.env in both of those files… it looks like v5 does some magic to write a fake process object when it’s missing globally
  • i think it would be preferable if mobx didn’t write to the global process object at all. sometimes, other programs might check the existence of that object to detect node environment, so if the solution is like mobx@5 here, mobx@6 could accidentally interfere with another system’s environment detection, causing tricky bugs

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 1
  • Comments: 27 (15 by maintainers)

Most upvoted comments

@chase-moskal The mobx 6.0.3 now includes ESM bundles without NODE_ENV. It was somewhat painful to convince tsdx about it, but I’ve managed.

add mobx.esm.production.js and mobx.esm.development.js to dist set package.json module field to mobx.esm.production.js (maybe setting jsnext:main and react-native too?)

We can do the first thing, but the second thing would break it for bundlers as I explained in https://github.com/mobxjs/mobx/issues/2564#issuecomment-718242870. You will have to set mapping for both cases unfortunately until someone comes with standardized package fields for this use case.

it may eventually be worth considering a serious refactor in this area, may or may not involve tsdx

Well, considering you the first one to be concerned by this compared to thousands of people going with “old way”, I don’t think that will happen anytime soon 😉

Thanks for the writeup anyway. It still sounds like a horrible developer experience to me, but it’s fine if you are satisfied. I’ve already included this in the checklist for the upcoming monorepo (#2530) where we will unify this across packages. I hope you can cope with V5 for now 😃 Closing and please follow the linked issue.

Fun fact, because esm is a live binding v8 can share memory across v8 isolates for esm in the same way it does for all the built in objects. Its kind of a big deal, if you have a lot of iframes or embeds or 2 tabs on the same realm. Just make a es5 target with esm syntax , its such an easy fix.

@FredyC

I get that, but I am trying to figure out what’s different from using current UMD vs browser compatible ESM.

Anyway, I have a feeling this whole thing brings yet another annoying fork into the ecosystem.

i’m worried i’ve confused you, so let me try to be less verbose here

i’m really just asking for mobx to support web browsers, the officially standardized way, via script tag, just like any other normal es module in the modern ecosystem

<script type=module>

  // MOBX FAILS: process is not defined
  import * as mobx from "https://unpkg.com/mobx@6.0.1/dist/mobx.esm.js"
  
  // but other modules just work
  import * as lithtml from "https://unpkg.com/lit-html@1.3.0/lit-html.js"

</script>

this technique is now how many junior developers are being taught to load dependencies. our familiar old bundling tooling is no longer required to load dependencies, thus making optional the tooling and advanced optimizations which interest devs like us

upgrading from mobx 5 to 6 broke my app, because for the last year, my apps have been loading dependencies standard way like above to achieve an awesome developer experience

i’m only asking for mobx to add this basic browser support. i’m not asking for anything existing to be removed or even changed

tsdx

i’d love to, but i just now cannot afford the time to sufficiently investigate or cross-reference the workings of mobx and tsdx


@urugator

okay. when looking at mobx/dist/ i now see what you’re saying: let’s see if we’re on the same page about this

for umd we have prod/min/dev

  • mobx.umd.production.js
  • mobx.umd.production.min.js
  • mobx.umd.development.js

for cjs we have prod/min/dev

  • mobx.cjs.production.js
  • mobx.cjs.production.min.js
  • mobx.cjs.development.js

but for esm we have something different

  • mobx.esm.js

and the proposed fix, is to publish similar prod/min/dev bundles for esm

  • mobx.esm.production.js
  • mobx.esm.production.min.js
  • mobx.esm.development.js

being pre-optimized, those new esm bundles won’t have any of the node-specific instructions, and thus should be browser-safe. i think this approach should fix this issue outright, leaving no breaking changes, making everybody happy 😃

thoughts?

  👋 chase

PS — i can’t afford to write a PR now, but i’ll try to be available for discussion and review. cheers!

Browser ESM will be a big thing I expect soon. A big benefit is that it makes the development cycle much more efficient as it doesn’t need bundling (I think snowpack does this already iirc) and it’s interesting for edge caching as well. Feel free to open a PR to configure the build to generate the .browser version? Curious were we’d run into. Note that you presumably want a minified version as well.

Op di 27 okt. 2020 21:49 schreef Daniel K. notifications@github.com:

I can’t really imagine what is the benefit of loading ESM in the browser. Generally, the ESM is useful as it’s statically analyzable and can be trimmed down only to things you have referenced in other modules. However, in the browser, you have to fetch the whole thing from the network anyway, so it seems to be losing that benefit?

That said, if you would use the UMD bundle we have, what would you lose there?

And I wonder if you use TypeScript in your when loading modules this way? It feels like it’s too detached from the actual code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx/issues/2564#issuecomment-717560426, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBGRC3SHC2I7NJ4V2FLSM455XANCNFSM4S764IMA .

@FredyC

okay, i understand. so mobx has special tooling-specific instructions in the module that breaks in web browsers. in the current case, mobx supports bundlers, not browsers

but keep this in mind: this issue is a recent regression in mobx@6, as mobx@5 works fine. so i’ve downgraded my apps to mobx@5 in the meantime

Honestly, I see this for the first time to load ESM in a browser like that. I would consider that an edge case. It’s not widely adopted for sure.

hah, i know it seems unusual. i mean, we’re all experts who are so familiar with our powerful tooling like rollup, npm, and the rest… now that we have these mastered, why imagine doing anything differently? well, as usual, things are changing on the web…

i work with cutting edge practices so that i can be a canary the coal mine. younger developers will be finding the new official recommended techniques from MDN and WHATWG, and it looks like this

<script type=module src="mobx6-is-broken.js"></script>

it’s no hipster edge-case. it’s just how javascript works now and moving forward.
since it’s so simple and lean, it’s how many developers will be taught

when a young developer sees that mobx fails to load, and the official explanation from the mobx team is: “no no silly, mobx doesn’t ‘just work’ as a javascript module… first, you have to install node, and npm, and establish a build toolchain starting with a rollup config, and don’t forget the node-resolver, and and and”… this just can’t be where we land

fixing this issue for mobx will prevent many more devs from making the same confused groaning noises as me yesterday when upgrading mobx broke my app 😉

possible solution: add module mobx.universal.js

  • environment agnostic (works everywhere, browsers, node, and deno)
  • perhaps suffers of bloat? (bundler couldn’t optimize out naughty bits, right?)

here’s a different solution

  • mobx.module.js — no change, has the bundler-specific instructions for optimizations
  • mobx.browser.js — new module is optimized for browsers (should work for deno too)
  • mobx.node.js — new module is optimized for node

i suppose one or both of those solutions could be employed. however it’s perhaps worth a deeper reconsideration of the general way mobx is optimizes itself for target environments

another way to look at this, is that mobx currently only supports bundlers (and, i guess, node): but i’m asking for mobx to at least add browser support, or even ideally, offer universally-agnostic support for all javascript environments

let’s make mobx work in browsers. we can do it!

    👋 chase

You have to import either: mobx.esm.production.min.js for production mobx.esm.development.js for development

@FredyC — i’ll try my best to explain

here’s my simplified scenario, let me know if this makes sense

  • my browser app consumes es modules
  • my app is using import maps to instruct the browser where to find packages
    • thus, i can configure an alias to point mobx at the development bundle whenever necessary
  • my app has a development build and a production build
    • in production, my app would assume the default module entrypoint for mobx would naturally point at mobx.esm.production.js
      • the package.json module field would need to be set this way
    • in development, my app would alias mobx to point at mobx.esm.development.js

so from my perspective, the procedure to fix this issue might look something like

  • add mobx.esm.production.js and mobx.esm.development.js to dist
  • set package.json module field to mobx.esm.production.js (maybe setting jsnext:main and react-native too?)

also, i have an idea about the process object shenanigans

  • as you know, i’m concerned about mobx having a global side effect like this: we’re dressing up the window object in a node-looking dress so we can fool bundler conventions into thinking we’re a particular node environment — what’s scary about that, is that we might get more than we bargain for, and start fooling other javascript programs on the page, which may then malfunction in tricky ways
  • it may eventually be worth considering a serious refactor in this area, may or may not involve tsdx
  • however, at the very least, we can at least strive to clean up the mess we make! if we must write a process object, let’s do it only while mobx is loading — then afterwards, we set the global back to whatever it was before we started loading mobx — this way, we can save the day by using the process object hack, and we can be polite about it — win win!

@mweststrate

I’m curious how you are loading react, because they don’t offer an esm build either.

since i’ve moved on from react last year, i’ve been having a stellar time writing 100% esm apps and true web components with lit-element and mobx with the help of @adobe/lit-mobx

i’d say lit-element is an excellent example of a well-formed esm library in the modern ecosystem worth emulating. they’ve achieved a marvelous minimalism and set the bar higher than most. very interestingly, they refuse to distribute any bundles, or minifications, or environment-specifics in a library, identifying those as footguns bad for consumers… so instead, lit-element only distributes simple pristine es modules… in the longer term, i’d really like to see mobx@7 take inspiration from these simpler practices, go esm-first, cut away some obsolete complexity, and join as a proud leader of our newly emerging javascript ecosystem… food for thought! 😃

anyways, back to the primary issue, i think we’ll generally agree on this solution:

  • write the process object (then clean it up afterwards!) in the two new esm bundles so browsers don’t crash
  • and set package.json ‘module’ field to the production bundle

    👋 chase

Sorry for wrong tone, just writing style no ill feelings. I dont load using cdn like chase is trying to, i rebundle deps as esm using rollup and exclude all eternals in application code (to reap the benefit of shared v8 heap). I was just making the point there is a huge runtime benefit (that few people know about) for practically 0 effort of publishing a minified esm bundle for cdns, although i personally have no use for it, a lot of people do.

V8 can optimize esm imports in the same way the builtins are optimized as discussed here. https://v8.dev/blog/embedded-builtins

I think that defaulting process.env to an empty object (as in Mobx5) should be quite safe and it’s convinient even with bundler - no need to configure “replace” to get started. Or is it somehow problematic?