adraffy / ens-normalize.js

ENSIP-15 in JS
https://adraffy.github.io/ens-normalize.js/test/resolver.html
MIT License
63 stars 17 forks source link

Name collision with process #23

Closed Javarome closed 9 months ago

Javarome commented 11 months ago

When building with Vite a web application that uses ens-normalize (typically indirectly through some ethers dependency) and a process polyfill, a runtime error occurs about the process identifier being already defined in the global scope:

image

Indeed, process is both a global NodeJS constant and a global function in this package's code.

While I agree that including a (polyfilled) process dependency in a webapp is a bad idea, a number of libraries do this, and renaming or scoping the process() function might help integrating ens-normalize more easily in many environments.

adraffy commented 11 months ago

Ah, thanks. I'll rename this function and double-check for any other node or related name collisions.

I will update this thread again once I apply these changes.

adraffy commented 11 months ago

Is your project using the default Vite bundler? This seems like a bug with esbuild.

Javarome commented 11 months ago

Yes default bundler. To reproduce, you can:

  1. checkout the example here
  2. npm install
  3. npm run dev
  4. open http://localhost:5173
  5. check error in the the console
adraffy commented 10 months ago

Sorry for the delay. I renamed process() but it will be fixed in NPM on the 12th when I deploy the Unicode 15.1 update.

In the meantime, I have the following solution for your vite.config.js:

import { NodeGlobalsPolyfillPlugin } from '@esbuild-plugins/node-globals-polyfill'
import {defineConfig} from "vite"
import {readFile} from 'node:fs/promises'; // required import statement

export default defineConfig({
  optimizeDeps: {
    esbuildOptions: {
      define: {
        global: 'globalThis'
      },
      plugins: [
        {                                                                    //
          name: 'raffy-fix',                                                 // 
          setup(build) {                                                     // plugin
            build.onLoad({ filter: /ens-normalize/ }, async args => {        // that
              let contents = await readFile(args.path, {encoding: 'utf8'});  // renames
              contents = contents.replaceAll('process(', 'raffy_process(');  // process()
              return {contents};                                             // to
            });                                                              // raffy_process()
          },                                                                 //
        },                                                                   //
        NodeGlobalsPolyfillPlugin({buffer: true})
      ]
    }
  }
})

Alternatively, you could just skip adding this temporary plugin and enable optimizeDeps.minifyIdentifiers: true, which will mangle my function (but might cause issues with other libraries.)

Javarome commented 10 months ago

Indeed, it works this way. Can't wait for your latest version to be integrated in ethers! Thank you so much for the change and the workaround!

ricmoo commented 10 months ago

Let me know when it’s published and I will make a patch release of ethers v6. :)

adraffy commented 10 months ago

This rename is active in the latest npm release 1.10.0