Skip to content

Resolve relative paths for preprocessor styles - #5829

Merged
ianschmitz merged 1 commit into
react:masterfrom
unrevised6419:resolve-url-loader
Aug 6, 2019
Merged

Resolve relative paths for preprocessor styles#5829
ianschmitz merged 1 commit into
react:masterfrom
unrevised6419:resolve-url-loader

Conversation

@unrevised6419

@unrevised6419unrevised6419 commented Nov 16, 2018

Copy link
Copy Markdown

preprocessor will output sourceMap by default
then check if sourceMap are needed on resolve-url-loader

issue demo: https://github.com/iamandrewluca/cra-scss-relative-problem

image

Fixes#4653

@stale

staleBot commented Dec 16, 2018

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stalestaleBot added the stale label Dec 16, 2018
@unrevised6419

Copy link
Copy Markdown
Author

@gaearon@Timer any fedback on this please? :)

@stalestaleBot removed the stale label Dec 17, 2018
@Timer

Copy link
Copy Markdown
Contributor

Sorry this fell through the cracks! I'm going to tag it for a milestone and hopefully get back to it soon. 😄

@TimerTimer added this to the 2.1.x milestone Dec 23, 2018
@yazeedb

Copy link
Copy Markdown

I ran this change on an internal TypeScript app, and can confirm it resolves the issue.

Might fork react-scripts to use it for now, but getting this merged would be sweeeeeeet.

@Silic0nS0ldier

Copy link
Copy Markdown

Hi everyone, any progress on this? Would be nice to clear out the dependency resources that have had to be moved into a project I'm working on. However the longer this goes unfixed, the less likely that is to happen.

@unrevised6419

Copy link
Copy Markdown
Author

Rebased

unrevised6419 pushed a commit to roataway/roataway-web that referenced this pull request Mar 8, 2019
Revert eslint changes when react/create-react-app#6513
will be merged
Revert to react-scripts when react/create-react-app#5829
will be merged
@unrevised6419

Copy link
Copy Markdown
Author
  • Rebased.
  • Updated resolve-url-loader to last version.

@iansuiansu modified the milestones: 2.1.x, 3.xMar 10, 2019
@jackwilsdon

Copy link
Copy Markdown
Contributor

This seems to have issues when using NODE_PATH, as the path resolves to a relative path outside of src;

src/index.scss

@import'~example/scss/index.scss';

vendor_modules/example/scss/index.scss

body {
background: url('../logo.svg');
}

Output from yarn build with NODE_PATH=./vendor_modules;

ModuleNotFoundError: Module not found: Error: You attempted to import ../vendor_modules/example/logo.svg which falls outside of the project src/ directory. Relative imports outside of src/ are not supported.
You can either move it inside src/, or add a symlink to it from project's node_modules/.

@unrevised6419

unrevised6419 commented Mar 20, 2019

Copy link
Copy Markdown
Author

@jackwilsdon this is not related to current issue. CRA has this restrictions from the beginning. If CRA would allow relative sources that fall outside src this error should not happen.

@jackwilsdon

Copy link
Copy Markdown
Contributor

My apologies @iamandrewluca, I misunderstood this PR as being a potential fix for #5402, which is caused by sass-loader not knowing the full path to the file and instead assuming it's not in NODE_PATH or src.

@unrevised6419

Copy link
Copy Markdown
Author

@jackwilsdon no problem. I added a reference in #5402 (comment)

@DalerAkhmetov

Copy link
Copy Markdown

thank you for this pull request. It also be useful for working with Font Awesome 5 icon library.

There is some code in SASS file located, for example, in '/app/src/styles.scss':
@import "~@fortawesome/fontawesome-free/scss/fontawesome"; @import "~@fortawesome/fontawesome-free/scss/solid"; // this file contains "url('#{$fa-font-path}/fa-solid-900.eot')"
This sass file will generate error: Module not found: Can't resolve '../webfonts/fa-solid-900.eot' in '/app/src'.

@mrmckebmrmckeb self-assigned this Jun 25, 2019
@mrmckeb

Copy link
Copy Markdown
Contributor

Hi all, sorry this fell through the cracks. I'll follow this up for now on.

@iamandrewluca - are you still interested in working on this? If so, let me know and I'll work with you to get it in.

@unrevised6419

Copy link
Copy Markdown
Author

@mrmckeb yes, still interested.

@mrmckebmrmckeb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, great. Let me take a look at this over the next few days.

I can see that you've included a demo which is great.

One small comment, otherwise it looks good.

Comment threadpackages/react-scripts/config/webpack.config.js Outdated
@unrevised6419

Copy link
Copy Markdown
Author

@mrmckeb done!

@mrmckebmrmckeb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ianschmitz or @iansu - any concerns on this one?

@mrmckebmrmckeb modified the milestones: 3.x, 3.1Jul 24, 2019
@mrmckeb

Copy link
Copy Markdown
Contributor

Still waiting for feedback on this one, but I'd like to get it out in 3.1. @ianschmitz was away, but hopefully he can take a look this week.

@ianschmitzianschmitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a quick change otherwise looks good.

Comment threadpackages/react-scripts/package.json Outdated
preprocessor will output sourceMap by default
then check if sourceMaps are needed on resolve-url-loader
Fixes#4653
@ianschmitz
ianschmitz merged commit 914c95e into react:masterAug 6, 2019
@locklockBot locked and limited conversation to collaborators Aug 11, 2019
@unrevised6419
unrevised6419 deleted the resolve-url-loader branch December 5, 2019 20:23
Sign up for freeto 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.

Cannot resolve relative references in nested .scss files with react-scripts@2.0

10 participants

@unrevised6419@Timer@yazeedb@Silic0nS0ldier@jackwilsdon@DalerAkhmetov@mrmckeb@ianschmitz@iansu@facebook-github-bot