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

Derequires build to be friendly with browserified projects #17

Merged
merged 1 commit into from
Dec 1, 2015

Conversation

dmarcos
Copy link
Contributor

@dmarcos dmarcos commented Nov 9, 2015

Thanks for your hard work. Your library has been very useful. I'm running into a problem when using your module within a project that also uses browserify. It's unfortunate that browserified dependencies don't play well within a browserified project! The parser looks for the require calls in all the modules (including external dependencies) and tries to resolve them. We end up with a bunch of Cannot find module XXX.js errors. That's an old issue that as far as I know hasn't been solved yet. There's a discussion going here: browserify/browserify#1151

On your side, you can run your build through derequire to make it easier to consume. derequire renames the require calls to _dereq_ so browserify ignores them. No functionality is affected. The only downside is that your build file will take a tiny bit longer to generate.

@borismus
Copy link
Contributor

Hmm... this seems like a real hack.

Is there some better JS module system? Maybe a good time to look into ES6 modules?

@dmarcos
Copy link
Contributor Author

dmarcos commented Nov 15, 2015

This is a pretty common approach to handle this. The real problem is that Browserify doesn't properly deal with this problem by itself. If you have any other idea I will update the PR

@jensarps
Copy link

A first option could be to just not use browserify to create the build. Closure Compiler does a good job (from my experience) handling CJS modules – and it will get rid of all require() calls in the output, resolving the issue.

Is there some better JS module system? Maybe a good time to look into ES6 modules?

As this project is targeted exclusively at browsers, and is never going to run on a server, I have the impression that using synchronous CJS as module format might not be the best choice. Going for ES6 seems like a good idea; but, it would require yet-another-extra-step during both development time, and for the build.

A second option that comes to my mind is to switch to AMD module format. The main benefit of this would be that no build step and watchers were needed during dev time (i.e. get rid of the watchify dependency). Do your changes to the code, reload in the browser, done. With regards to the build, Closure Compiler can also handle AMD modules well (again, just from my experience).

In any case, I’d wrap the resulting build in a UMD wrapper to make sure ppl can consume it the way they like; but, that is something else, as it might mean that using make isn't sufficient anymore and one would need to switch to Grunt or Gulp.

If there is interest, I could give one or both options a shot over the weekend.

@borismus
Copy link
Contributor

@dmarcos sorry for delay. Looking at this again. Can you link to how you use webvr-polyfill with require?

@dmarcos
Copy link
Contributor Author

dmarcos commented Nov 30, 2015

I have nothing special in my config. The problem is very easy to reproduce:

  1. Create an empty directory
  2. Copy inside the webvr-polyfill dist file
  3. Create a main.js file with just this line require('./webvr-polyfill.js');
  4. Run the following command (this would be the browserify step within my project that would require my own files)
    browserify ./main.js -o ./myLib.js
  5. Do you see the error? Error: Cannot find module './base.js'
    Browserify is trying to resolve the required files by your browserified build

Now try to run your file through derequire (npm install -g derequire) and try again:

  1. derequire webvr-polyfill.js > webvr-polyfill.js
  2. browserify ./main.js -o ./myLib.js
  3. Now it should work

@borismus
Copy link
Contributor

Have you considered using https://www.npmjs.com/package/browserify-derequire ?

@dmarcos
Copy link
Contributor Author

dmarcos commented Dec 1, 2015

Yes, this is what I'm doing and it's fine. This PR makes a preemptive derequire on webvr-polyfill (at 0 cost) side so users are not surprised (as I was) and have to eventually realize that you're also using browserify and need to add a derequire step. It's unfortunate that browserify doesn't handle this in a more graceful way because it's a common use case.

borismus added a commit that referenced this pull request Dec 1, 2015
Derequires build to be friendly with browserified projects
@borismus borismus merged commit c8cec0d into immersive-web:master Dec 1, 2015
@borismus
Copy link
Contributor

borismus commented Dec 1, 2015

Thanks, sorry for delay. Merged.

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

Successfully merging this pull request may close these issues.

4 participants