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.

Sign up for GitHub

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

Jump to bottom

Seems like livereactload is making paths absolute #123

Open
sassanh opened this issue Aug 10, 2016 · 12 comments
Open

Seems like livereactload is making paths absolute #123

sassanh opened this issue Aug 10, 2016 · 12 comments
Labels
bug

Comments

@sassanh
Copy link

sassanh commented Aug 10, 2016

Suppose that I run this:

browserify a.js -r ./scripts/components/b.js -o c.js

Now a.js can require b.js by calling require('/scripts/components/b.js'). But if I add -p livereactload to the above that require will fail. Reading the compiled code I tried require('/Users/x/y/z/w/scripts/components/b.js') (the absolute path in my filesystem) and it worked. It seems like livereactload is somehow making paths absolute in compiled file. Is it possible to avoid this?
@milankinen
Copy link
Owner

Sorry for late delay, not been able to look at GH over week. This definitely looks like a bug. Which LiveReactload version causes this issue?

@milankinen milankinen added the bug label Aug 19, 2016
@sassanh
Copy link
Author

sassanh commented Aug 19, 2016

Thanks for following this up. I'm using 2.3.0-rc7.

@milankinen
Copy link
Owner

I just tried this on my own computer and it seems that this might be a watchify issue. The following code causes the same error:

watchify a.js -v -r ./tmp/b.js -t babelify -g envify -o bundle.js

@sassanh can you confirm this on your side?

@sassanh
Copy link
Author

sassanh commented Aug 24, 2016 •
edited
Loading

@milankinen Thanks for your attention. I described the problem incomplete and incorrect. I'm so sorry about that.
Here's the instruction on how to reproduce it:

1.Create a.js like this:

require('./test/b.js').test()

2.Create test/b.js like this:

module.exports.test = function() { console.log('it's b') }

3.Create a.html like this:

<html>
    <head>
        <script src='d.js'></script>
        <script src='c.js'></script>
    </head>
</html>

4.Compile these files this way:

node_modules/.bin/browserify a.js -x ./test/b.js -o c.js
node_modules/.bin/browserify -r ./test/b.js -o d.js

5.Serve the files, I use python: python -m SimpleHTTPServer 8009
6.If you open localhost:8009/a.html in browser it prints "it's b" in console.
7.Now compile b.js with node_modules/.bin/browserify -r ./test/b.js -o d.js -p [livereactload]
8.If you open same url, it'll show an error: "c.js:1 Uncaught Error: Cannot find module '/test/b.js'"

I exposed require to window.r so that I can access it via console and after spending a lot of time and reading compiled d.js and c.js I were able to require b.js using its absolute path for example "/Users/x/y/z/b.js". I compared the compiled file with and without livereactload and found out it's somehow changing the "id" section of the module, this is from d.js compiled with livereactload:

      "id": "/Users/x/y/tmp/test/b.js",
      "hash": "5be5e9f6f92c535b8a251cf912f578b0",
      "browserifyId": "/test/b.js"

This absolute path does not exist in the compiled file without livereactload. I can't remember the exact procedure I followed to be able to require it via its absolute path, but it's not that important as all we need is being able to require it via its relative path: "./test/b.js".

If you change browserify to watchify in above compile commands, it still works.

Sorry for long comment and sorry again for incomplete and incorrect description I provided earlier.

P.S. Currently I've solved the issue for myself by compiling all files into a single bundle and simulating lazy loads in dev environment and using multiple bundles only in production environment where I don't compile files with livereactload.

@milankinen
Copy link
Owner

Wow, exceptionally well documented bug report! Thank you very much @sassanh! It really seems that the bug is inside LiveReactload. With these informations, I think I'm able to to do the fix without any problems.

@sassanh
Copy link
Author

sassanh commented Aug 26, 2016

Glad to hear that. I try to document bugs for others the way I want them documented for myself :D

@sassanh
Copy link
Author

sassanh commented Aug 27, 2016

I don't know if it's related to this issue, I don't know if it's specific to my setup too. When I use livereactload with -d it ruins the sourcemaps a little bit, there are 2 problems:

  1. files with same name override each other: if I have two files with name reducer.js and if an error occurs in first file, if I click on the location chrome shows in the error traceback (reducer.js:40 for example) it may go to the other reducer.js. It doesn't change in a single load of page, but after each reload it's random.
  2. Lines are slightly changed, for example it shows the error happened in line reducer.js:40 but it really happened in line 36 for example. It's always a few lines but sometime I need to re compile code without livereactload to be able to debug.

Another thing that seems weird and may be related is that my bundle is around 7.5MB without livereactload but with livereactload the size reduces to around 6MB.

I just thought maybe these are also related to this thread. Let me know if I should create another issues for these and if I should provide additional information :)

@milankinen milankinen mentioned this issue Sep 16, 2016
Closed
@milankinen
Copy link
Owner

Thanks! I created another issue for these source map issues: #127

@sassanh
Copy link
Author

sassanh commented Sep 16, 2016

OK! Thanks!

@arcticShadow
Copy link

Is there any news on this issue? Or anyway to assist?

@arcticShadow arcticShadow mentioned this issue Nov 23, 2016
Closed
@EugeneZ
Copy link
Contributor

EugeneZ commented Nov 24, 2016

@arcticShadow Check out PR #138 . It might help these problems. If not, please reply in the PR since I think it's related.

milankinen added a commit that referenced this issue Nov 25, 2016
@milankinen
Merge pull request #138 from EugeneZ/master
158f8cd 
Fixes #127 #123
@milankinen
Copy link
Owner

Released 3.1.1 which should fix the source maps bug. However, the original absolute path issue may remain so still keeping this open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@EugeneZ @milankinen @sassanh @arcticShadow