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

CSS - Search in parent folders for node module resolution #81555

Conversation

penx
Copy link
Contributor

@penx penx commented Sep 27, 2019

Replaces my previous PR #79651 by implementing recommendations from @octref on #78894 .

Further to #70693 which was reworked by @aeschli in 8f72934 due to issues with require.resolve and webpack.

The current implementation for microsoft/vscode-css-languageservice#136 resolves the path to a CSS module but only looks for node_modules in the same folder as the CSS file, which for many use cases is not sufficient.

This PR updates resolvePathToModule so that it looks in parent folders for the existence of node_modules/module-name all the way up to the workspace root, following node module resolution.

Alternative approaches were raised in #79651 and this approach was preferred by @octref in #78894 (comment).

Screenshot 2019-08-23 at 16 18 58
Screenshot 2019-08-23 at 16 19 48

@penx
Copy link
Contributor Author

penx commented Jan 5, 2020

@octref @aeschli is there anything holding up merging this PR? Any more I can do?

@Tyriar Tyriar modified the milestones: November 2019, January 2020 Jan 6, 2020
@octref octref modified the milestones: January 2020, February 2020 Jan 29, 2020
@Tyriar Tyriar modified the milestones: February 2020, March 2020 Mar 10, 2020
@penx
Copy link
Contributor Author

penx commented Mar 23, 2020

@Tyriar any more I can do to get this in to a release?

@Tyriar
Copy link
Member

Tyriar commented Mar 23, 2020

@penx I have nothing to do with this, I simply pushed it out as it didn't make it into the release.

@penx penx changed the title Search in parent folders for node module resolution CSS Modules - Search in parent folders for node module resolution Mar 23, 2020
@penx penx changed the title CSS Modules - Search in parent folders for node module resolution CSS - Search in parent folders for node module resolution Mar 23, 2020
@penx
Copy link
Contributor Author

penx commented Mar 23, 2020

#20200323.20 failed
BackupRestorer Restore backups:
11980
Error: Backup: Could not find meta end marker in vscode

After rebasing off master, the above error is reported. Pretty sure this is unrelated to this PR and tests should be rerun

@octref octref modified the milestones: March 2020, April 2020 Apr 1, 2020
@aeschli
Copy link
Contributor

aeschli commented May 5, 2020

@penx Sorry for the wait.
Can you also add a test case in https://github.com/microsoft/vscode/blob/master/extensions/css-language-features/server/src/test/links.test.ts ?

@aeschli aeschli requested review from aeschli and removed request for octref May 5, 2020 09:12
@aeschli aeschli self-assigned this May 5, 2020
@aeschli aeschli modified the milestones: April 2020, May 2020 May 5, 2020
@penx penx force-pushed the feature/node-module-resolution-for-css-import-parent-folders branch from 3875853 to 237c803 Compare May 6, 2020 13:33
@penx
Copy link
Contributor Author

penx commented May 6, 2020

@aeschli no worries, I've added a test and reordered commits to show test fail then pass

@aeschli aeschli merged commit 815d94e into microsoft:master May 14, 2020
@aeschli
Copy link
Contributor

aeschli commented May 14, 2020

Thanks @penx!

@penx penx deleted the feature/node-module-resolution-for-css-import-parent-folders branch May 14, 2020 15:03
@penx penx restored the feature/node-module-resolution-for-css-import-parent-folders branch May 14, 2020 15:15
@github-actions github-actions bot locked and limited conversation to collaborators Jul 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants