JustinGOSSES / wellioviz

d3.js v5 visualization of well logs
https://justingosses.github.io/wellioviz/#introduction
Apache License 2.0
51 stars 12 forks source link

Not compatible with react 17.0 #127

Closed 40uf411 closed 2 years ago

40uf411 commented 2 years ago

Describe the bug This library depends on wellio. Even if I am not using wellio, wellioviz imports it anyway. The problem is that wellio uses "fs" which is not meant for web browsers. Therefore, every time I run the react server it crushes saying "fs" not defined in wellio/dist... I tried yarn and npm, they both showed the same error

To Reproduce Steps to reproduce the behavior:

  1. Create a react app
  2. install wellioviz
  3. Run the server: npm start or yarn start
  4. See error

Expected behavior A clear and concise description of what you expected to happen. Running normally, as it does with Browsefy.

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Smartphone (please complete the following information):

Additional context Add any other context about the problem here.

JustinGOSSES commented 2 years ago

Thanks for flagging this issue @40uf411 .

  1. Can you expand on the what you mean when you say "create react app"? For example are you running npx create-react-app my-app as described on https://create-react-app.dev/ ?

There's more than one way to set up a react project, so this will help me replicate your error.

  1. Can you describe the steps you use to install wellioviz and where those steps are being taken in your react project structure?

Again, this will allow me to replicate the error exactly.

  1. Can you look at your package.json or package-lock.json file and check to see what version of wellioviz and wellio is being used?

This will help debugging. Certain code was added to only use 'fs' when wellio was being called on the server side and not part of a front end project. That code has changed a little other time, so version information will help debugging.

  1. Can you post the exact error message.

This may help confirm what is calling fs.

JustinGOSSES commented 2 years ago

Just wanted to report that I was able to get the same error by:

I then get in the browser window at http://localhost:3000/ the error message

Compiled with problems:X

ERROR in ./node_modules/wellio/dist/index.js 76:13-26

Module not found: Error: Can't resolve 'fs' in '/Users/justingosses/Code/wellioviz_testing/my-app/node_modules/wellio/dist'

However, I will note that If write a scrip that says console.log("THIS IS A TEST OF CONSOLE LOG ALSO with wellioviz",wellioviz.help()) as first line of the detault app() function in src/App.js that it does print the correct message in the console of I'm really no help. Please check out the docs at https://justingosses.github.io/wellioviz/ or the main README.md at https://github.com/JustinGOSSES/wellioviz.

I'm assuming this is very likely the same error for the same reason.

So it looks like wellioviz installs correctly but wellio.js tries to do something that attempts to use or call or install 'fs' , which it shouldn't be doing.

JustinGOSSES commented 2 years ago

As this may be an issue entirely on the wellio side, adding an issue other there https://github.com/JustinGOSSES/wellio.js/issues/76

However, exact cause has let to be determined so keeping issue open in both places for now.

JustinGOSSES commented 2 years ago

Why 'fs' fails on the lines mentioned in the file is a bit strange as it doesn't seem tied together.

For reference, this is link to all issues on wellio that mentions 'fs'. Will try to pick this back up later. https://github.com/JustinGOSSES/wellio.js/issues?q=is%3Aissue+fs

40uf411 commented 2 years ago

I apologize for the late reply. I assume you fine a way to replicate the error. As you said, wellio is using fs when it shouldn't. However, Js is not really my cup of tea and can't say for sure where the problem is.

JustinGOSSES commented 2 years ago

Had a little bit of time to spend on this. Haven't figured out yet where the error is coming from. fs isn't on the line mentioned in the error message and even if you comment out all mentions of 'fs' in the file node_modules/wellio/dist/index.js, it still shows the error for 'fs' when running the development server via npm start.

Needless to say, this is a little weird.

However, if you build the project, the error goes away.

Run npm run build and then start a local server to see the build. The default way to do this is npm install -g serve to install serve and then serve -s build.

When I do this, I don't see the error in the final built version. The error only appears in the development server version.

This isn't a solution, but it probably means in the short term, it is possible you might be able to just ignore that error as fs shouldn't be used in front-end projects anyway. I didn't do check all of wellioviz's capabilities but a quick check in the console.log messages seemed to work okay.

I'll try to look more later.

dcslagel commented 2 years ago

It has been a long time since I looked at the Well* code. I remember we tried to wrap code to check whether the running environment was Browser or Node. In a browser the code should set to fs to ''. , and then use new FileReader to read files. If the code is running in a Node environment then it should set fs = require('fs');.

The browser specific code is in Wellio.js's js/main.js The browser code is roughly:

   var files = document.getElementById("files").files                                                                                                                                                     
    if (files) {                                                                                                                                                                                           
      for (var i = 0, file; file = files[i]; i++) {                                                                                                                                                        
       var reader = new FileReader();                                                                                                                                                                     
        reader.readAsText(file, "UTF-8");    
       ....
   }               

The code that sets fs to empty by default and then only run require('fs') when the code thinks it is NOT in a browser environment is in wellio's dist/index.js

     loadLAS: function (wellLog) {
      const file = wellLog;
      let fs = '';

      if (process !== 'undefined' && process.versions != null && process.versions.node != null) {
        // eslint-disable-next-line global-require
        fs = require('fs');
      }
      const contents = fs.readFileSync(file).toString();
      // var contents = fs.readFileSync('test.LAS', 'utf8');
      return contents;
    },
 ...

So maybe something about the interaction of this code and the React Server is letting the Javascript parser get to the js = require('fs');. There may be some addition condition that can be added or a refactoring that will keep the Server and Browser from seeing fs and let Node environments continue to use fs.

Another solution might be to use new FileReader; in both Browser and Node environments, if it is possible...?

JustinGOSSES commented 2 years ago

Thanks for your comment @dcslagel .

I had forgot that main.js has some 'fs' relevant code, but it doesn't get in the wellio package when used as a dependency of wellioviz as its not in wellio's dist folder.

I'm wondering if the code if (process !== 'undefined' && process.versions != null && process.versions.node != null) runs partially effectively server side when using the development server and but effectively front-end when running from a built version? Not sure if that's a thing, but the local servers used are at least some what different? Mostly putting this here as an idea to check into later.

JustinGOSSES commented 2 years ago

Looks like if we add this statement process.env.NODE_ENV != 'development' as well so that line in wellio's index.js file is if (process !== 'undefined' && process.versions != null && process.versions.node != null && process.env.NODE_ENV != 'development') { The error doesn't appear on development server either.

I'll go ahead and submit the PR to wellio once I get a few minutes, but it is working when I make the changes locally in the node_modules folder of a testing react project.

JustinGOSSES commented 2 years ago

Appeared to have fixed it in wellio's pull request 83 . Ran tests and appears to work okay now.

Released new version: https://github.com/JustinGOSSES/wellio.js/releases/tag/v0.1.13