ES module browser build expects "process" global

[guybedford]

When running this package on jspm, it will give "process is undefined" because of the process check in the index.browser.js file.

The assumption of the browser environment shouldn't be based on the proces global existing. Adding a typeof process !== 'undefined' check would fix this issue.

[ai]

@guybedford can you send PR with typeof process !== 'undefined' to save your time in the history?

[TrySound]

process.env.NODE_ENV will never be reached because of typeof process for example in rollup when bundling for browser.
This is widely used convention and it's not specific to node packages. "browser" field does not mean process.env.NODE_ENV cannot be used.

[guybedford]

@TrySound to be super clear here, while index.browser.js in this repo is CommonJS, the published version is published as an ES module - https://unpkg.com/nanoid@3.1.12/index.browser.js. It is the combination of assuming the process global, setting this as the entry for the browser platform, and then also shipping it as an ES module that is specifically the issue here. Any of the lesser combinations wouldn't be as bad!

[ai]

As a temporary solution you can use import { nanoid } from "nanoid/nanoid.js"

[guybedford]

@ai the problelm is a dependency of a dependency importing nanoid is broken when respecting the "browser" exports field breaking all packages on jspm which depend on nanoid. It would be preferable not to add the "browser" field if it isn't going to be a valid ES module for the browser.

[ai]

Let’s create an issue in jspm to add process.env.NODE_ENV support

[guybedford]

@ai the problem is that process is not a browser global - therefore an ES module that expects it to exist will break in the browser. jspm follows browser semantics so will not fix this.

[guybedford]

Only when using CommonJS will jspm support the process global by default.

[ai]

@guybedford do you see any other option on how to keep good DX and support browser API?

I am suggesting changing jspm because I do not see other options.

[guybedford]

@ai I'm not sure why the original form of this PR to add typeof process wasn't a suitable option to be honest.

[ai]

Because webpack doesn't add process to the system. It looks for the exact process.env.NODE_ENV call and replace only this code to "production" or "development" string:

if (typeof process !== "undefined" && process.env.NODE_ENV === "development") {

Will be compiled to:

if (typeof process !== "undefined" && "development" === "development") {

Since browser do not have process it will be false && true.

[guybedford]

I see, that makes sense.

I think the root issue here is that you're taking an ES module designed to match CommonJS module semantics for Webpack semantics, then stating it to be the browser entry point when it simply isn't a native browser module at all.

I would suggest authoring a native browser module directly that fits the requirements of the browser, rather than starting from the above constraint.

[guybedford]

Alternatively, simply removing the "browser" entry for now, or exactly pointing the "browser" entry to the CommonJS file could be an option too.

[ai]

simply removing the "browser" entry for now,

We need to separate Node.js and browser file, because they have a different crypto API

Why adding process.env.NODE_ENV support to jspm is not an option for you? Anyway it converts require and __dirname and many other Node.js specific code.

Even if we will fix Nano ID we will have many and many npm modules with process.env.NODE_ENV.

[guybedford]

Many of us are actively working towards workflows where the "browser" ES module can run in the browser with just an import map, and everything else based directly on the native standards. This is what it means to have a native module system in the browser.

If such a module now expects arbitrary globals to exist in the host environment that don't otherwise exist, then you need a build step.

Certainly many are on the side of living with builds in JS for any package loaded, and perhaps that is the philosophical difference here. But jspm stands firmly on the native platform module side and won't budge on that.

[ai]

If such a module now expects arbitrary globals to exist in the host environment that don't otherwise exist, then you need a build step.

We will need some kind of build step for performance reasons.

Every for pure ESM code we will need:

1. Tree-shaking
2. Minification
3. Dependencies map for preload tag generation

Certainly many are on the side of living with builds in JS for any package loaded, and perhaps that is the philosophical difference here. But jspm stands firmly on the native platform module side and won't budge on that.

I like the idea of a revolution for good ideas.

But my experience tells me that there are two kinds of revolutions:

1. Denying any other ideas and user’s experiences
2. Making your new world welcome for new users

In my experience, the first type of revolutions fail.

How we will be able to create a new world by ignoring all content of the npm? Why do we need jspm build-on-the-fly CDN if it does not allow us to use npm library in ESM projects?

Do you remember Python 2→3 revolutions with compatibility drop? They still didn’t end this migration. If we will use only pure solutions, ESM will not survive and migration to ESM will take decades for the community.

There are many use cases when we want to keep the production bundle from unnecessary development checks. There are plenty of development-only checks in React. In Nano ID that if (process.env.NODE_ENV === "development" contains many helpful warnings, but we can’t put it to production bundle.

We need some way to do it. And more important, that we need a way to convert all packages.

[guybedford]

jspm as a project builds significantly on the ability to allow users to load any package from npm with full npm CommonJS and semantic compatibility in a way that requires no build. There are a lot of deep details around how it handles package optimization, treeshaking, import maps and preloading generation which I can't go into in a GitHub comment. If you're interested in discussing this side of things further I'm always happy to chat.

That all to say that most of the work is exactly about building ecosystem bridges here. From SystemJS to Node.js modules. Python 2-3 comes up a lot in Node.js modules discussion, I like to think we avoided those biggest issues in having a full compatibility story.

As for the issue at hand there are a few main points:

1. As one of the first packages to set the "exports" "browser" field to an ES module, this is not about compatibility, this package itself is now one of the first to be setting these future conventions in fact.
2. The "development" and "production" conditions in the exports field are fully supported in Webpack and jspm. And they can even be configured in Node.js via the --conditions=development flag. This is a pattern that is not widely known about so I think it is an important one to start evangelising now.
3. Inline code production condition checks are still an important use case, and there have been discussions in the past about a new universal pattern here like import.meta.env or import.meta.production. Unfortunately there has been no progress on this as there is no central place to make import.meta suggestions and Node.js doesn't want to make any moves here. Perhaps this is something Webpack and build tools could adopt as an ad-hoc convention which could then get greater momentum.

Unfortunately for the process.env.NODE_ENV inlining, you cannot always assume that users will also define typeof process to be truthy and I can appreciate that is a hinderance.

I was thinking about this some more though and I think it can work with the separate development.js and production.js files if you remove the process.env.NODE_ENV check entirely and entirely use the exports split here to achieve this as with (2) above, something like:

{
  "exports": {
    "browser": {
      "production": "./index.browser.js",
      "default": "./index.browser.development.js"
    },
    "require": "./index.cjs",
    "import": "./index.js"
  }
}

With the pattern in https://github.com/guybedford/nanoid/blob/patch-1/index.browser.development.js.

If you would be open to that I would be glad to reopen the PR - let me know? I just couldn't see where you are populating the "exports" field though through dual-publish.

For (3) I think it would be useful to have further discussion here on this topic.

The definition of the "browser" field in exports is that the module runs in the browser. I believe Webpack 5 will not be automatically shimming process either, but am not sure.

I think naturally any package that wants to support the browser will smooth these things out over time. The more we can remove the implicit weird vestigal assumption from the new modern workflows we are creating the better, but I will certainly concede on compat arguments if the main river goes that way, but we still have time on this specific convention to see which way things go.

[ai]

Yeap. Send a PR with an another way to avoid development checks in production bundle.

I like exports.browser.production more since process.env.NODE_ENV is really hack.

[guybedford]

Sure, I've created a tracking issue in dual-publish for this at ai/dual-publish#14.

[ai]

I released 3.1.17 with new dual-publish. Now Nano ID has index.prod.js and index.dev.js with additional package.exports.browser = { development, production } key.

Does it work in your case?

[guybedford]

Looks great, and its working perfectly with jspm now thanks.

[ai]

@guybedford how do you test it with jspm? I want to add test to dual-publish?

[guybedford]

The local installer is still in private beta so isn't available for testing yet. When it is, a local test run would be possible. In the mean time for post-publish testing import('https://jspm.dev/nanoid') or deno eval "console.log(await import('https://jspm.dev/nanoid'))" can work, although there's few mins delay before those reflect after publishing so it's harder to automate right now.

[ai]

I need some tool to test unreleased packages.

OK, feel free to ping me on any regressions with dual-publish’s packages like nanoid.

[ai]

@guybedford oops, seems like jspm takes pkg.exports.development instead of pkg.exports.production: https://jspm.dev/npm:nanoid@3.1.18

Look at if (true) {

[guybedford]

Yes, the .dev in jspm.dev is because it is a development server and shouldn't be used for production workflows, so will always resolve the development conditions. The production CDN is the unreleased beta which then points to https://ga.jspm.io/npm:nanoid@3.1.18/index.prod.js.

If you are interested in trying it out locally, the private beta is open to anyone - you can request an invite from Discord here https://discord.com/invite/dNRweUu.