Plant-Tracer / webapp

Client and Server for web-based JavaScript app
GNU Affero General Public License v3.0
0 stars 2 forks source link

Modules in CommonJS format don't love the browser yet #566

Open sbarber2 opened 1 week ago

sbarber2 commented 1 week ago

planttracer.js:680 Uncaught ReferenceError: module is not defined at planttracer.js:680:1

Getting the above error message in the browser console on every page.

Here's the offending line:

module.exports = {list_movies_data, list_users_data, register_func, upload_movie_post}

This was most likely very recently introduced in #422

joedinsmoor commented 1 week ago

Taking a look...

joedinsmoor commented 1 week ago

It's a weird node thing, figured out the issue, fixing now

joedinsmoor commented 1 week ago

I have a fix, building and testing now.

edit: Here's the deal, the exports from planttracer.js are in CommonJS format, which isn't usually supported by browsers. This means that the modules that are being exported need to be transpiled to ES6 modules, which are supported. However, this will break jest testing. To fix jest testing, we would need to add a couple things to package.json as well as change .babelrc/babel.config.js, which break other modules that are unrelated to planttracer.js.

Specifically the line "type": "module" breaks other imports in the project when added to package.json

sbarber2 commented 1 week ago

I have an idea for a really stupidly simple short-term workaround if doing it The Right Way is getting complicated:

if (typeof module != 'undefined') {
    module.export(......)
}

I tried this. All the jest tests still passed, and the browser error went away.

joedinsmoor commented 1 week ago

Beautiful, lets do that until I have a way to fix this.

edit: right now, ESM support is experimental and unstable in Jest, according to their documentation, I believe that we probably will need to use your runaround until they have a stable way of transforming.

sbarber2 commented 1 week ago

Actually, let's re-open this and leave it there because we're going to be dealing with this again someday.

joedinsmoor commented 1 week ago

Going to leave this here as a resource for future reference: https://jestjs.io/docs/ecmascript-modules