olalonde / eslint-import-resolver-babel-root-import

A babel-root-import resolver for eslint-plugin-import
MIT License
23 stars 33 forks source link

Update babel plugin name #6

Closed adrianmcli closed 7 years ago

adrianmcli commented 7 years ago

Fixes #5

Linting and tests all pass.

adrianmcli commented 7 years ago

@pcguru I was having trouble with switching over so I just left it. I can take a look at it again.

adrianmcli commented 7 years ago

I've further illustrated my trouble with that in this issue on the babel-plugin-root-import repo: https://github.com/entwicklerstube/babel-plugin-root-import/issues/66

zurfyx commented 7 years ago

That's a breaking change!

IMO, you should either update package.json name as well and republish it, just like babel-root-import did. eslint-import-resolver-babel-plugin-root-import would make sense.

Or you should at least jump an entire version — to 1.0.0 — rather than a subversion like it is now. You don't want to break current app linters who are targeting ^0.0.2

adrianmcli commented 7 years ago

@pcguru It seems that they suggest we keep using the old version. https://github.com/entwicklerstube/babel-plugin-root-import/issues/66#issuecomment-283379803

@zurfyx good point, I will do this.

adrianmcli commented 7 years ago

I'll probably fork and republish if this isn't merged in sometime in the next few weeks.

GollyJer commented 7 years ago

@adrianmcli is your branch working correctly? I'd love to get a working version of this bad boy if possible. I tried simply updating the name in .eslitnrc but no luck.

adrianmcli commented 7 years ago

@GollyJer yeah it should be working, I've been using it for the past few weeks. Give it a go and let me know.

GollyJer commented 7 years ago

@adrianmcli Thanks for the help. I can't get it to work. Clearly I don't know what I'm doing.

Here's what I tried. "eslint-import-resolver-babel-root-import": "git://github.com/adrianmcli/eslint-import-resolver-babel-root-import.git#718368a629ef7c181a12a647224823faba5ac5d3"

.eslintrc

settings: {
    'import/resolver': {
      'babel-plugin-root-import': {
        rootPathPrefix: '~',
        rootPathSuffix: './_app',
      },
    },
  },

.babelrc

"plugins": [
    [
      "babel-plugin-root-import", // https://github.com/entwicklerstube/babel-plugin-root-import
      {
        "rootPathPrefix": "~",
        "rootPathSuffix": "./_app"
      }
    ]
  ],
adrianmcli commented 7 years ago

@GollyJer you can see how I use it here: https://github.com/adrianmcli/next-boilerplate

GollyJer commented 7 years ago

@adrianmcli Working perfectly, thanks! It looks like others are taking a crack at it too.

adrianmcli commented 7 years ago

@GollyJer haha, maybe it's time that I fork and publish an actual working option.

GollyJer commented 7 years ago

@adrianmcli yeah, take a look at the version by @rmacklin. It seems like he's done a bunch of clean up too.

rmacklin commented 7 years ago

Whoa, wasn't expecting anyone to see my fork, haha. I got that fork working on Thursday and was thinking about publishing it, but then I ended up switching approaches and dropping babel-plugin-root-import for a symlink inside node_modules because I liked the simplicity of that approach (which makes things like eslint and editor import detection "just work"). That said, I'd be happy to combine efforts on an updated version of this plugin.

A few things to note about your (@adrianmcli's) changes:

But another thing I realized is that the getConfigFromBabel function doesn't handle if the plugin is only declared in the env section of your .babelrc. For that reason, one change I was trying on my fork was dropping getConfigFromBabel altogether. Instead, the options would be passed explicitly to the eslint plugin config (which can still be DRY if you're using .eslintrc.js and .babelrc.js with babel 7).

michaeljonathanblack commented 7 years ago

@rmacklin Could you provide a link to details on the symlink approach? I'm migrating a client app that used webpack path resolving to use server-side rendering and I need a server-workable path solution. Relative paths are just a nightmare with our feature-focused directory structure.

Thanks in advance!

Best Michael

michaeljonathanblack commented 7 years ago

Ooh, some details around symlink, here: https://gist.github.com/branneman/8048520

A bummer that it's a solution that can't quite be committed, though.

rmacklin commented 7 years ago

@mherodev, a symlink can be committed actually, although I don't do that. Instead, I create the symlink in a postinstall script (and remove it in a preinstall script, so that it doesn't exist during the actual installation step, since that can cause issues).

michaeljonathanblack commented 7 years ago

@rmacklin I don't hate the preinstall/postinstall script approach.

I took the solution available on the linked gist and updated it to use fs.stat over the deprecated fs.exists call:

    "preinstall": "node -e \"var d='node_modules/src',fs=require('fs');fs.stat(d,function(e){!e&&fs.unlinkSync(d)});\"",
    "postinstall": "node -e \"var s='../src',d='node_modules/src',fs=require('fs');fs.stat(d,function(e){e&&fs.symlinkSync(s,d,'dir')});\"",

This seems pretty straightforward and works. Thanks for the insight!

rmacklin commented 7 years ago

Seems like that'd work 👍 . In my project I just run a bash script rather than doing it in node, since I'm fortunate to not have to worry about cross-platform inconsistencies.

camsjams commented 7 years ago

Any plans on this being merged or published elsewhere under a new name?

olalonde commented 7 years ago

Anyone wants to be added to the repos' maintainers? I'm no longer using this plugin.

GollyJer commented 7 years ago

We're still using. I can take if you don't mind.

olalonde commented 7 years ago

@GollyJer added you as a collaborator on GitHub

srghma commented 7 years ago

Can someone merge and publish it finally?

adrianmcli commented 7 years ago

@BjornMelgaard I think @GollyJer is your best bet right now.

srghma commented 7 years ago

@GollyJer, you are collaborator now, can you publish this fix)) P.S. @adrianmcli, thanks for boilerplate example, finally got it working)

GollyJer commented 7 years ago

I'm on vacation until the 12th. Will merge this asap.

GollyJer commented 7 years ago

This is now merged and released. I've never published to NPM. Is there a simple way to pass that torch or will you need to do it @olalonde?

adrianmcli commented 7 years ago

@GollyJer glad to see this finally merged. You'll need access to publish the package on NPM. @olalonde will likely have to give you permission to do this. If he already did, then just bump the version number (while following SemVer) and do npm publish.

If you've never published anything on NPM before, I highly recommend you publish a test project just so you can see how the process works.

Thanks again for merging!

P.S. For those that want to use this immediately, you can replace the version number in your package.json with a link to this repo.