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

Crazy relative paths for some people #233

Closed
lencioni opened this issue Mar 18, 2016 · 12 comments
Closed

Crazy relative paths for some people #233

lencioni opened this issue Mar 18, 2016 · 12 comments
Labels

Comments

@lencioni
Copy link
Collaborator

I've been working with some folks to get import-js set up, and I've seen this happen with @ljharb and @iancmyers. It is a bit puzzling to me, and I have not figured out how to reproduce in my environment, but here's what I know.

Symptoms

ImportJS seems to work pretty okay, except when resolving relative paths, we end up seeing a LOT of ../../../. It looks like it goes all the way up to / and then back into the project. This is very obvious when there are multiple modules to pick from, and definitely not what we want to be happening. This includes files that should be in the same directory (e.g. ./foo comes up as `../../../../../../path/to/the/thing/foo').

@ljharb

Jordan is using Sublime, rvm, and nvm. He has no system node installed.

@iancmyers

Ian is using Atom, rbenv, and I'm not sure about other details.

These are all happening in the same project that I am working with, and are using version 0.6.0 of the gem.

@lencioni lencioni added the bug label Mar 18, 2016
@trotzig
Copy link
Collaborator

trotzig commented Mar 19, 2016

Interesting. I'd be surprised if it goes all the way to the root, because we strip out (at least from what it looks like) the path up until (and including) the project path. It would be interesting to see what ruby versions they use, though I doubt that's the problem.

In order to track this down, it would be great if you could try a few things:

  • Try importing for the same file using the CLI tool (something like import-js path/to/file.js --word foo, where path/to/file.js is the file you are editing and --word foo is the variable you are trying to import that's causing the ../../../../../ thing to happen)
  • Modify the make_relative_to method in lib/import_js/js_module.rb using gem open import_js and add some debug output:
    # @param make_relative_to [String]
    def make_relative_to(make_relative_to)
      return unless lookup_path

      # Prevent mutating the argument that was passed in
      make_relative_to = make_relative_to.dup

      # First, strip out any absolute path up until the current directory
      make_relative_to.sub!("#{Dir.pwd}/", '')

      # Ignore if the file to relate to is part of a different lookup_path
      return unless make_relative_to.start_with? lookup_path

      # Strip out the lookup_path
      make_relative_to.sub!(%r{^#{Regexp.escape(lookup_path)}/}, '')

      puts 'DEBUG INFO'
      puts lookup_path
      puts Dir.pwd
      puts import_path
      puts make_relative_to

      path = Pathname.new(import_path).relative_path_from(
        Pathname.new(File.dirname(make_relative_to))
      ).to_s

      # `Pathname.relative_path_from` will not add "./" automatically
      path = './' + path unless path.start_with?('.')

      self.import_path = path
    end

@lencioni
Copy link
Collaborator Author

I should have mentioned that when I looked at this with @ljharb, we tried the CLI tool and everything was good there, which makes me think that there might be something weird going on with the cwd. I wonder if it might be vagrant or symlink related.

Update: I was unable to reproduce through symlink shenanigans.

@ljharb
Copy link

ljharb commented Mar 19, 2016

As for Ruby versions, this was the same with system 2.3 and 2.0 (working on the CLI, not in sublime). It didn't work in 1.9.3 at all.

@lencioni
Copy link
Collaborator Author

@ljharb can you try adding the debugging code and reporting back here what
it produces?

On Sat, Mar 19, 2016, 3:27 PM Jordan Harband notifications@github.com
wrote:

As for Ruby versions, this was the same with system 2.3 and 2.0 (working
on the CLI, not in sublime). It didn't work in 1.9.3 at all.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#233 (comment)

@ljharb
Copy link

ljharb commented Mar 20, 2016

Where does that debug output end up?

@lencioni
Copy link
Collaborator Author

It should appear at the top of the file you are editing

On Sat, Mar 19, 2016, 5:20 PM Jordan Harband notifications@github.com
wrote:

Where does that debug output end up?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#233 (comment)

@ljharb
Copy link

ljharb commented Mar 20, 2016

ok so while trying to add the debug code, I ended up removing every copy of the import_js gem, and reinstalling it just under system. My sublime settings point to /usr/local/bin/import-js.

I can now use "goto module" in sublime somewhat (sometimes it opens a new file at the path but without the extension) but the linter still doesn't work, and when i select "fix all", i get:

Error when executing import-js:

/usr/local/lib/ruby/gems/2.3.0/gems/import_js-0.6.0/lib/import_js/importer.rb:190:in `run_eslint_command': env: node: No such file or directory (ImportJS::ParseError)
    from /usr/local/lib/ruby/gems/2.3.0/gems/import_js-0.6.0/lib/import_js/importer.rb:78:in `fix_imports'
    from /usr/local/lib/ruby/gems/2.3.0/gems/import_js-0.6.0/bin/import-js:66:in `<top (required)>'
    from /usr/local/bin/import-js:23:in `load'
    from /usr/local/bin/import-js:23:in `<main>'

Let's debug this in person on Monday :-)

@trotzig
Copy link
Collaborator

trotzig commented Mar 20, 2016

Thanks for being so patient @lencioni and @ljharb.

@trotzig
Copy link
Collaborator

trotzig commented May 1, 2016

It would be interesting to see if this is still an issue with the new Node binary.

@lencioni
Copy link
Collaborator Author

lencioni commented May 1, 2016

Yeah, I'm hoping we've resolved this. We are currently blocked on wbond/package_control_channel#5463 being merged.

@janpaul123
Copy link
Contributor

Doesn't seem to be resolved for me! 😮

trotzig added a commit that referenced this issue May 11, 2016
We had code in place which would strip out the path to the current
directory from the path to the current file. That bit of code wasn't
working, since we were feeding the String.replace with a string instead
of a regexp.

This resulted in relative paths not working correctly in Sublime (which
passes in absolute paths for pathToCurrentFile). We could make that
plugin also pass local paths, but I figured it was worth fixing this in
the main project anyway, because the command-line tool needs to support
these paths.

Fixes #263, which is likely a duplicate of #233.
@trotzig
Copy link
Collaborator

trotzig commented May 11, 2016

I believe 599a684 should fix this issue. Sorry for the long wait.

@trotzig trotzig closed this as completed May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants