`process is not defined` error occurring when using the CDN source

chacha912 commented 6 months ago

What happened:

In PR#717, a Webpack configuration(nodeEnv: false) was added to make process.env.NODE_ENV apply in the environment where the library is used. However, this caused the process.env.NODE_ENV to not be replaced with a string in the built code and remain exposed as is. Consequently, when using the CDN source, the process is not defined error occurs in the browser environment.

We need to investigate how to apply a different Webpack configuration to the source deployed on the CDN. (Ref: https://github.com/cdnjs/packages/pull/737)

What you expected to happen:

When using the CDN source code, errors should not occur. Devtools should be available when using the yorkie-js-sdk.js source, but unavailable when using the yorkie-js-sdk.min.js source.

How to reproduce it (as minimally and precisely as possible):

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Document</title>
  </head>
  <body>
    <div>There are currently <span id="peersCount"></span> peers!</div>

    <!-- include yorkie js -->
    <script src="https://cdnjs.cloudflare.com/ajax/libs/yorkie-js-sdk/0.4.13/yorkie-js-sdk.js"></script>
    <script>
      async function main() {
        const client = new yorkie.Client('https://api.yorkie.dev', {
          apiKey: '', // enter your key 
        });
        await client.activate();

        const doc = new yorkie.Document('my-first-document');
        doc.subscribe('presence', (event) => {
          const peers = doc.getPresences();
          document.getElementById('peersCount').innerHTML = peers.length;
        });
        await client.attach(doc);
      }
      main();
    </script>
  </body>
</html>

Anything else we need to know?:

Environment:

Operating system:
Browser and version:
Yorkie version (use yorkie version):
Yorkie JS SDK version: 0.4.13

blurfx commented 5 months ago

It seems like the problem could be easily solved by type checking like we've been doing. I'll create a PR.

chacha912 commented 5 months ago

@blurfx It seems I was a bit short on context in explaining the issue. In order to make devtools work only in the development build environment, I added the condition: (process.env.NODE_ENV === 'production').

However, adding the condition typeof process !== 'undefined' resolves the error in CDN environments, but it causes an issue where devtools always run in environments using a JavaScript bundler.

You can verify this by installing the yorkie-js-sdk from the examples and then running devtools.

During npm run dev, devtools should be running.
During npm run build -> npm run preview, devtools should not be running. (Currently, due to the condition typeof process !== 'undefined', devtools still run in the production build environment.)

Is there a good way to resolve this?

For reference, Liveblocks provides the following guidance on this issue: https://liveblocks.io/docs/platform/troubleshooting#process-not-defined

blurfx commented 5 months ago

I think we need to set environment variables or configure bundler plugin to determine the environments.

Let me check it out a bit more. I'll reopen the issue until it's resolved.

chacha912 commented 5 months ago

@blurfx Thanks. Currently, to avoid this issue, we've changed Devtools execution to be determined directly by users setting true/false instead of checking process.env.NODE_ENV. (ref: https://github.com/yorkie-team/yorkie-js-sdk/pull/760)

const doc = new yorkie.Document('docKey', {
  enableDevtools: true, // Modify the condition according to your situation
});

Once this issue is resolved, it would be good to reintroduce the feature of Devtools defaulting to running only in development builds.

blurfx commented 2 months ago

@chacha912 It's been a few months since enableDevtools config was added. Do you still think it's a good idea to make development tools available in development builds without extra config like enableDevtools? If yes, I want to investigate more and work on it.

chacha912 commented 2 months ago

@blurfx Yes, I think it's still a good idea. Having devtools enabled by default in the development environment would likely make the development process more convenient. (The enableDevtools option would still be available for scenarios where more fine-grained control is needed.) This change would allow us to remove enableDevtools from the examples in the public folder. Thank you.

blurfx commented 2 months ago

@chacha912 then I'll work on enabling devtools by default in the development environment, and optionally using the enableDevtools for detail scenarios.

hackerwins commented 2 months ago

@blurfx @chacha912 Thanks for your interest in this issue.

JS SDK provides two installation methods, and developers can either import it as an npm module or include it directly on their web pages using a CDN link.

https://yorkie.dev/docs/getting-started/with-js-sdk#setup

When using the SDK via npm, we have the ability to control devtools activation based on whether it's a development or production build, typically using the process.env.NODE_ENV.

However, when the SDK is included directly via CDN, there's no build process involved. We need to consider this case, find the good way to enable devtools in this scenario. Also, be careful to avoid script errors caused by process.env.NODE_ENV.

yorkie-team / yorkie-js-sdk

`process is not defined` error occurring when using the CDN source #768