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

Set baseUrl from jsconfig.json/tsconfig.json #6656

Merged
merged 22 commits into from
Apr 16, 2019
Merged

Set baseUrl from jsconfig.json/tsconfig.json #6656

merged 22 commits into from
Apr 16, 2019

Conversation

rovansteen
Copy link
Contributor

@rovansteen rovansteen commented Mar 15, 2019

This is a partial implementation of #6116. Because there are still a number of things that need to be figured about alias support I think it will be the best approach to add support for baseUrl first.

This will be a breaking change and removes support for setting NODE_PATH in .env file in favor of setting baseUrl in either jsconfig.json or tsconfig.json. It adds a warning to the start script to inform the user about this change if there is a NODE_PATH value in process.env.

This PR aims to add the existing behavior of resolving modules with absolute path that already exists in CRA but in a consistent manner for both TypeScript and JavaScript projects.

Since this is a breaking change it would be nice to get this in with 3.0. After that we can create another PR to add support for aliases (like @) since that would be a non-breaking change.

packages/react-scripts/scripts/start.js Outdated Show resolved Hide resolved
test/fixtures/typescript/tsconfig.json Show resolved Hide resolved
@zheeeng
Copy link

zheeeng commented Mar 16, 2019

cheers, we neeeeeed it!

@rovansteen
Copy link
Contributor Author

rovansteen commented Mar 16, 2019

Not sure if (or how) the tests on the CI are failing because of my changes 🤔

@eladmotola
Copy link

eladmotola commented Mar 16, 2019

@rovansteen for the tests that failed on install:
change modules.js :
const chalk = require('react-dev-utils/chalk');
instead of - const chalk = require('chalk');

I think it should fix it.
I know its not much but its something I guess... btw thank you for that!

@rovansteen
Copy link
Contributor Author

@eladmotola thanks, that fixed the installs suite!

@iansu
Copy link
Contributor

iansu commented Apr 16, 2019

@rovansteen I've fixed the behavior tests. There are a couple of last minute questions from @mrmckeb and myself and then I think we're ready to merge this.

@rovansteen
Copy link
Contributor Author

@iansu thanks! I’m away for a few days now, will be back on Thursday. Then I can look at the question from @mrmckeb.

@iansu iansu merged commit e7a2d61 into facebook:master Apr 16, 2019
@iansu
Copy link
Contributor

iansu commented Apr 16, 2019

Thanks for everyone's work on this, especially @rovansteen!

@delaaxe
Copy link

delaaxe commented Apr 17, 2019

Congrats!

@rovansteen
Copy link
Contributor Author

Thanks for merging this and helping out with the final bits @iansu! 👏🏻

@mrmckeb
Copy link
Contributor

mrmckeb commented Apr 17, 2019

This works wonderfully, thanks @rovansteen. I think a lot of people are going to be very grateful for your work here!

@iansu
Copy link
Contributor

iansu commented Apr 17, 2019

Now we just need someone to document this before the 3.0 release: #6765

@g1eny0ung
Copy link

@rovansteen Thanks to your jobs. 🎉🎉

@nasreddineskandrani
Copy link

Thanks for the effort. Really Really appreciated add on.

@FezVrasta
Copy link
Contributor

So do we have to import from 'src/xxx' or can we do as before? (Without src)

@rovansteen rovansteen deleted the typescript-base-url branch April 22, 2019 21:27
@rovansteen
Copy link
Contributor Author

@FezVrasta setting the baseUrl to src allows you to import directly from src without prefixing it as such. So if you have src/components/button.js you can import it anywhere with import Button from 'components/button'.

For more information check out the documentation.

@delaaxe
Copy link

delaaxe commented Apr 23, 2019

@rovansteen How do we import using import 'src/components/xxx' instead? Is there a supported way to configure that without resorting to a separate *.paths.json file?

@zheeeng
Copy link

zheeeng commented Apr 23, 2019

Can we define more customized path aliases like '@src', '@components', '@config', '~src', '~components', '--utils' which have been supported by Webpack and tsconfig.json many years and widely used?

@rovansteen
Copy link
Contributor Author

We are open to adding support for aliases in a future release. But right now it’s limited to baseUrl only.

@ConAntonakos
Copy link

We are open to adding support for aliases in a future release. But right now it’s limited to baseUrl only.

Awesome! This would be amazing, and I wonder if it'll be easier with this jsconfig.json config.

@vishal423
Copy link

In one of my sample projects, I use eslint-config-airbnb preset, and after migration to create-react-app v3.0, I have to also include below in .eslintrc configurations to resolve lint errors on absolute imports in vscode (please note that build and start scripts works fine). Is that documented or am I missing something?

  "settings": {
    "import/resolver": {
      "node": {
        "paths": ["src"]
      }
    }
  }

@BANG88
Copy link

BANG88 commented Apr 26, 2019

If you have a tsconfig.paths.json before you should remove it and put compilerOptions configurations inside tsconfig.json. This took me 20 minutes to make it work.

@silltho
Copy link

silltho commented Apr 26, 2019

@rovansteen How do we import using import 'src/components/xxx' instead? Is there a supported way to configure that without resorting to a separate *.paths.json file?

We are facing the same issue, does someone have an idea how to fix this?

@delaaxe
Copy link

delaaxe commented Apr 27, 2019

@silltho Check this comment it worked like a charm for me and others: #5118 (comment)

@lock lock bot locked and limited conversation to collaborators May 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.