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

Rollup #117

Open
theKashey opened this issue May 21, 2020 · 14 comments
Open

Rollup #117

theKashey opened this issue May 21, 2020 · 14 comments

Comments

@theKashey
Copy link
Owner

continuation of #113

@boneskull
Copy link
Contributor

OK, I'm looking at this now. I'm getting this:

Uncaught ReferenceError: require is not defined
    at eval (eval at <anonymous> (utils.js:30), <anonymous>:1:1)
    at utils.js:30
    at createCommonjsModule (4e9c7d39-5c4d-4d18-9319-4a0a6cc380de.rollup.js:21)
    at index.js:2368
    at 4e9c7d39-5c4d-4d18-9319-4a0a6cc380de.rollup.js:4
    at 4e9c7d39-5c4d-4d18-9319-4a0a6cc380de.rollup.js:5

That error is thrown from L30 in lib/plugins/common/utils.js.

require does exist, but it is not called require (it's commonJsRequire), so eval'ing it does not work.

However, working around that by replacing eval('require') with true causes the same issue as in #113; commonjsRequire.resolve is not a function, coming out of node-libs-browser.

Mocha doesn't call require.resolve() in browser tests during usual operation. Anything that we might use from Node.js is already polyfilled via rollup-plugin-node-builtins or explicitly (e.g., browser-stdout or other monkeypatching).

I hope this is useful. Here's a gist of the resulting bundle

@theKashey
Copy link
Owner Author

Well the solution is straightforward:

export const ensureRequire = 
// Is it webpack?
typeof __webpack_require__ !== 'undefined' 
// yeah? So let's use it's internal require
? __webpack_require__ : 
// no? Then 👉👉👉👉 let's hide this require from webpack 👈👈👈👈
eval('require');

It has a special logic around require only to suppress some problems with webpack, and it just should do something it is not doing right now.

Like detect rollup/browserify and do what they need. Or detect that "this is not webpack", and don't do some silly movements.
The problem is that should be static and declarative

  • could some process.env help here? There are no standards around which env could be defined
  • could some package.json main/browser magic help here? Not sure it could distinguish webpack from other bundlers using the default configuration (can with non default)
  • could some DI help here? Probably yes. Like entrypoint for webpack could import some helpers and assign them to modules, while node could load others, and rollup entry could do the same.

Technically it would not cause a major version change - I could create a new entrypoint which will work that way, keeping the old behavior default.


Try this

Could you please make ensureRequire = require and test if it works?

@boneskull
Copy link
Contributor

It won't, because we'll run into require.resolve is not a function, but I'll try

@boneskull
Copy link
Contributor

HeadlessChrome 84.0.4147 (Mac OS X 10.15.5) ERROR
  Uncaught TypeError: commonjsRequire.resolve is not a function
  at .karma/yoyodyne/d1892d42-be9d-4784-b59b-55a774d22d3c.rollup.js:37070:31

  TypeError: commonjsRequire.resolve is not a function
      at .karma/yoyodyne/d1892d42-be9d-4784-b59b-55a774d22d3c.rollup.js:37070:31
      at .karma/yoyodyne/d1892d42-be9d-4784-b59b-55a774d22d3c.rollup.js:4:28
      at .karma/yoyodyne/d1892d42-be9d-4784-b59b-55a774d22d3c.rollup.js:5:2

@boneskull
Copy link
Contributor

(rollup replaces require with commonjsRequire)

@theKashey
Copy link
Owner Author

🤔

Mocha.unloadFile = function (file) {
   delete commonjsRequire.cache[commonjsRequire.resolve(file)];
};

Sounds like I should expose "current" wipe method from https://github.com/theKashey/rewiremock/blob/master/src/wipeCache.js

@boneskull
Copy link
Contributor

unloadFile is not run by browser tests

@theKashey
Copy link
Owner Author

In this case the problem has origin in

addPlugin(plugins.nodeLibBrowser);

Not sure what this branch is used in your case, but it is what loads node-libs-browser

@boneskull
Copy link
Contributor

OK, I've made some progress here.

I discovered that Rollup doesn't really love the dynamic require in plugins/node-lib-browser.js--it treats it as a static require(). Using await import() might be an alternative, but I don't doubt that'd have other consequences.

So, I've told Rollup to ignore node-lib-browser entirely. Now, I get:

Uncaught Error: Rewiremock: there is no "parent module". Is there two HotModuleReplacementPlugins?
    at index.js:42
    at createCommonjsModule (38fe420c-c5c2-4460-94a2-94aa9c19f00e.rollup.js:21)
    at defaultConfig.js:22
    at 38fe420c-c5c2-4460-94a2-94aa9c19f00e.rollup.js:4
    at 38fe420c-c5c2-4460-94a2-94aa9c19f00e.rollup.js:5

rewiremock seems to be calling getModuleParent on what appears to be node_modules/rewiremock/lib/index.js. Of course, there is no parent property of the module. There is some support for dynamic requires--which would ostensibly populate module.parent with...something (maybe not anything useful)--but I'm still failing trying to get it to work.

@boneskull
Copy link
Contributor

Rollup doesn't seem to have the ability to handle dynamic modules as webpack does (afaict this is a design decision). I would imagine if we wanted a module "tree", rewiremock would need to implement it. 😦

@boneskull
Copy link
Contributor

I wonder if nollup could be helpful

@theKashey
Copy link
Owner Author

node-lib-browser is needed only to provide mapping to the node_modules overriden by webpack.
You might not need at all - please try to comment it's usage in rewiremock/lib/index.js

@boneskull
Copy link
Contributor

yes, I eliminated it entirely from the package, but then I get the HotModuleReloading err above

@theKashey
Copy link
Owner Author

Which is bound to the module relationship information, which is not quite exists in rollup as you've mentioned. And HotModuleReloading is required in case of webpack - it brings those information.

What's with HMR stuff in rollup? Could it save the day as well?

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