detrohutt / react-app-rewire-inline-import-graphql-ast

Add the inline-import-graphql-ast Babel plugin to your create-react-app app via react-app-rewired
MIT License
17 stars 2 forks source link

Supporting absolute paths from .env settings #2

Closed loganpowell closed 6 years ago

loganpowell commented 6 years ago

HI @detrohutt ! Really enjoying the ease of use of this library so far. I've got everything working, but ran into a snag. I use a .env variable (NODE_PATH=src/) so that I can use absolute paths from the src folder instead of ../../.. hell and the problems that stem from them (e.g., moving referencing files around breaks imports), but it seems that react-app-rewire-inline-import-graphql-ast doesn't recognize the .env config.

Any pointers?

detrohutt commented 6 years ago

Hey, thanks for trying this out! I actually just made this package for my own personal use the day I found out about react-app-rewired, so I haven't put much work into it other than getting it to work for my specific use case. I believe to support what you're asking for, I'd need to support it in the upstream package, babel-plugin-inline-import-graphql-ast .. I don't think that will be a problem, but I'm pretty busy with another project at the moment so I'm not sure how soon I'll be able to get that implemented, but I'll do my best.

loganpowell commented 6 years ago

Thank you for the quick response! If it's ok, can I keep the issue open? No hurry at all!

detrohutt commented 6 years ago

Of course! I’ll update you via this issue as soon as I have any news.

On Jan 21, 2018, at 6:40 PM, Logan Powell notifications@github.com wrote:

Thank you for the quick response! If it's ok, can I keep the issue open? No hurry at all!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

loganpowell commented 6 years ago

Sweet.

detrohutt commented 6 years ago

Hey @loganpowell I had a thought. If you could put together a simple reproduction app with an example .env file and broken import, I can try to address this sooner.

loganpowell commented 6 years ago

@detrohutt sorry for the delay. It's been a crazy day in DC. Here's a link to the reference in my code:

https://github.com/loganpowell/RAATS/blob/master/src/smarts/github/course-list/courses.jsx#L7

detrohutt commented 6 years ago

@loganpowell It looks like you're using the relative path for now so I assume that's working? What I was thinking is a test CRA app that just basically has a .env file with the relevant settings (is it just NODE_PATH=/src?) and then a file nested somewhere down in /src that tries to import a graphql file using an absolute import.

That way I can make changes to my packages in the test app and once it's working there, I'll know I've got it fixed for you.

I don't mind making the test app myself if needed. I just figured if you make it I'll know everything is set up as you intended because I don't use NODE_PATH myself.

Edit: The test app shouldn't need an actual graphql server. As long as I don't get any import-related errors in the build phase, I'll just assume it's working.

loganpowell commented 6 years ago

You bet! I'm using a CRA with rewired and a couple other libraries. Let me know if I'm getting what your asking for:

I can set up a new app with my .env setup and just a couple of pages, sans the rest of the libraries except for react-app-rewired and the apollo libraries, correct?

loganpowell commented 6 years ago

actually, I probably don't need the apollo libraries... just check to ensure the import statement works..

detrohutt commented 6 years ago

Yes that is correct. I updated my original reply shortly after sending it to say not to worry about any server code. I guess the email reply goes out immediately. If you view it on github you’ll see what I mean.

loganpowell commented 6 years ago

Got it! I will have this done tonight!

loganpowell commented 6 years ago

Alright, Here it is!

https://github.com/loganpowell/graphql_imports/blob/master/src/github/course-list/courses.jsx#L7

loganpowell commented 6 years ago

notice absolute paths work for the "test" import but not for the "query"

detrohutt commented 6 years ago

Perfect! I’m hoping to take a shot at fixing this tomorrow morning. Thanks for your help. 😀

loganpowell commented 6 years ago

Thank you! Take your time.

On Tue, Jan 23, 2018, 19:52 Alexander Roberts notifications@github.com wrote:

Perfect! I’m hoping to take a shot at fixing this tomorrow morning. Thanks for your help. 😀

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/detrohutt/react-app-rewire-inline-import-graphql-ast/issues/2#issuecomment-359983519, or mute the thread https://github.com/notifications/unsubscribe-auth/ADQBPy9Cwu71afKlwvH1p4uwVKnI6_NPks5tNn7VgaJpZM4Rl-7g .

detrohutt commented 6 years ago

I'm taking a look at this now. CRA gitignores .env by default so yours isn't in the repo. Would you mind either updating the repo or just copying and pasting the contents of your .env here?

loganpowell commented 6 years ago

ah, sorry!:

PORT=3006
NODE_PATH=src/
detrohutt commented 6 years ago

Hey @loganpowell I've made some progress. I was able to get my plugin to respect the NODE_PATH env var BUT the problem is the .env file is not taken into account while babel is running.. So there are a few options.

  1. I could try to 'automatically' find a .env file from within my plugin. I don't want to do this though since there could be several different files like .env, .env.local, etc.

  2. I could provide an option similar to NODE_PATH where you pass a list of folders to use for absolute path resolution, i.e. "src/:/components/", and then I could set NODE_PATH in my plugin to that. This still seems error-prone as you'd have to keep the option synced up with the contents of your other NODE_PATH var.

  3. I could provide an option that takes a path to a .env file, and parse that file, looking for a NODE_PATH var, and use that internally. This seems the most reasonable option so far.

  4. You could do what I did to make it work while I was testing, which is to set NODE_PATH at a level where it will be in scope when babel runs. The way I did it was by running NODE_PATH=src/ yarn start in the graphql_imports folder. Alternatively, you could change the start script in your package.json to include the var.

Honestly, parsing .env files seems a bit outside the scope of what my package (the upstream one) is intended for. To make it work in this offshoot package I'd still need to create the option in the upstream package for this package to utilize internally.

I'd be willing to consider implementing option 3, but since there's a pretty simple workaround, that wouldn't be high on my priority list. Please let me know your thoughts.

P.S. - I got it working on my end, but I haven't published anything yet so you won't be able to make it work as I've described just yet.

loganpowell commented 6 years ago

@detrohutt sorry for the delayed response! I'm down for whichever solution you can afford ("beggars can't be choosy") :) Just tell me what to do.

detrohutt commented 6 years ago

Good news! I got it working without any configuration necessary (other than what's already in your .env). :) Just upgrade this package to the newest version and you should be good to go! I did run into a hiccup while publishing and had to revert some changes and republish but I think it's working now. Let me know if you run into any problems.

loganpowell commented 6 years ago

You are the man!

On Thu, Jan 25, 2018, 21:37 Alexander Roberts notifications@github.com wrote:

Good news! I got it working without any configuration necessary (other than what's already in your .env). :) Just upgrade this package to the newest version and you should be good to go! I did run into a hiccup while publishing and had to revert some changes and republish but I think it's working now. Let me know if you run into any problems.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/detrohutt/react-app-rewire-inline-import-graphql-ast/issues/2#issuecomment-360667568, or mute the thread https://github.com/notifications/unsubscribe-auth/ADQBP9mGC51hV1T1Bs5N1Le83lp5wgftks5tOTpsgaJpZM4Rl-7g .

loganpowell commented 6 years ago

Works like a charm!!!!

🙏

detrohutt commented 6 years ago

Great! Thanks again for your help in fixing it. Enjoy!

loganpowell commented 6 years ago

Thank you @detrohutt for being awesome

detrohutt commented 6 years ago

Hey, just a heads up, you may want to check out the new section I just added at the top of the README. It could save you a lot of time and frustration.

loganpowell commented 6 years ago

I'm grateful :)