Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

if (module) { } can cause a potential error. #234

Closed
CodePsy-2001 opened this issue May 15, 2022 · 17 comments
Closed

if (module) { } can cause a potential error. #234

CodePsy-2001 opened this issue May 15, 2022 · 17 comments

Comments

@CodePsy-2001
Copy link

can cause Error: 'module' is not defined

https://github.com/PepsRyuu/nollup/blob/master/docs/hmr.md

if (import.meta.module) {
    module.hot.accept(() => {
        window.location.reload();
    });
}

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import.meta

sorry for my poor English.

@PepsRyuu
Copy link
Owner

PepsRyuu commented May 15, 2022

import.meta.module isn't valid. Use the following instead:

if (module && module.hot) {
   module.hot.accept(() => {
        window.location.reload();
    });
}

You can find further information here: https://github.com/PepsRyuu/nollup/blob/master/docs/hmr.md#adding-hot-support-to-app

@CodePsy-2001
Copy link
Author

CodePsy-2001 commented May 15, 2022

but then, it causes 'module' is not defined error when using rollup...

@PepsRyuu
Copy link
Owner

That shouldn't happen... Do you have a copy of your Rollup configuration and package.json?

@CodePsy-2001
Copy link
Author

CodePsy-2001 commented May 15, 2022

I appreciate your kindness.

I'm trying using rollup, nollup with pixiJS.

package.json

{
  "scripts": {
    "build": "rollup -c --environment BUILD:production",
    "watch": "rollup -cw",
    "start": "nollup -c"
  },
  "type": "module",
  "name": "kuroki-like",
  "version": "1.0.0",
  "private": true,
  "main": "bundle.js",
  "keywords": [
    "pixijs",
    "rollup"
  ],
  "devDependencies": {
    "@rollup/plugin-alias": "^3.1.9",
    "@rollup/plugin-commonjs": "^22.0.0",
    "@rollup/plugin-node-resolve": "^13.3.0"
  },
  "dependencies": {
    "nollup": "^0.19.0",
    "pixi.js": "^6.3.2",
    "rollup": "^2.73.0",
    "rollup-plugin-commonjs-alternate": "^0.8.0",
    "rot-js": "^2.2.0"
  }
}

rollup.config.js

import resolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import commonjsAlter from 'rollup-plugin-commonjs-alternate'; // for nollup
import alias from '@rollup/plugin-alias';

export default {
  input: 'src/index.js',
  output: {
    file: 'bundle.js',
    format: 'es',
    sourcemap: true, // for debug
  },
  external: [],
  plugins: [
    ...process.env.BUILD === 'production' ? [alias({
      entries: [{
        find: /^(@pixi\/([^\/]+))$/,
        replacement: '$1/dist/esm/$2.min.js',
      }, {
        find: 'pixi.js',
        replacement: 'pixi.js/dist/esm/pixi.min.js',
      }]
    })] : [],

    resolve({preferBuiltins: false}),
    // `preferBuiltins` is required to not confuse Rollup with the 'url' dependence that is used by PixiJS utils

    process.env.BUILD === 'production' ? commonjs() : commonjsAlter()
    // important for pixiJS 
  ]
};

@CodePsy-2001
Copy link
Author

You know, using rollup with hot reload is too slow, so I was trying to use nollup with my pixiJS project.
I wanted to write my project in es6 style.
pixiJS must need commonJS, because of it's inner code.

@CodePsy-2001
Copy link
Author

CodePsy-2001 commented May 15, 2022

now, in this case, import.meta.module is valid with rollup, not nollup.
and module is valid with nollup, not rollup..

@PepsRyuu
Copy link
Owner

I can't reproduce the error using the provided package.json and rollup.config.js. Can you provide a screenshot of the error message?

@CodePsy-2001
Copy link
Author

My entire project file:
https://drive.google.com/file/d/1Gpme4u7s2GFYrVqy4J2eEpZ3BZHR_zEc/view?usp=sharing

I am using serve -s to see my production, because of local CORS error.

@PepsRyuu
Copy link
Owner

Ah the error is in the compiled code. Gotcha. Having a look into it!

@CodePsy-2001
Copy link
Author

pixiJS v6 + rollup boilerplate (official): https://github.com/pixijs/pixijs/wiki/v6-Boilerplate

I used this boilerplate.

@CodePsy-2001
Copy link
Author

where? how????

@CodePsy-2001
Copy link
Author

How did you know the error was in compiled code(bundle.js)? How can I avoid errors?

@PepsRyuu
Copy link
Owner

PepsRyuu commented May 15, 2022

From the looks of it, you just need to use something to remove the module code. Usually the module.hot logic is handled by a third-party plugin that automatically inserts it and removes it in production. You have a few options here.

  • Use @rollup/plugin-terser to remove the code.
terser({
    compress: {
        dead_code: true,
        global_defs: {
            module: false
        }
    }
});

You can see this in the documentation here: https://github.com/PepsRyuu/nollup/blob/master/docs/hmr.md#additional-build-configuration-for-hmr

  • Alternatively you can create your own plugin to inject the code:
process.env.BUILD !== 'production' && {
    transform(code, id) {
        if (id.includes('index.js')) {
            return code + `
                if (module) {
                    module.hot.accept(() => {
                        window.location.reload();
                    })
                }
           `;
        }
    }
},

Personally I would choose the second option, as it prevents your code from containing bundler specific logic. :)

@CodePsy-2001
Copy link
Author

CodePsy-2001 commented May 15, 2022

Thank you so much.
By the way, I wonder why you failed to reproduce the error.

Is it not becuase of rollup.config.js or package.json ??

@PepsRyuu
Copy link
Owner

I didn't try to run the code, I thought you meant that Rollup was throwing an error in the terminal. 🙂

@CodePsy-2001
Copy link
Author

I see. Thank you very much for your help.

Even when I usually write differenct codes, I have a hard time separating the context(node/compiled) of JavaScript as needed.

I hope this will be a lesson for me. haha :D

@PepsRyuu
Copy link
Owner

We've all been there! Even experienced engineers I teach make the same mistake occasionally. I've have worked with many senior engineers over the years who struggle to configure Webpack. Wanting to understand how bundlers worked was one of the reasons I started this project. 🙂 If you ever get the chance, I highly recommend challenging yourself to write your own very basic bundler. It's one of my favourite exercises I like to give to any student of mine. 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants