Skip to content

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

Merged
Martin Aeschlimann (aeschli) merged 3 commits into
microsoft:masterfrom
penx:feature/node-module-resolution-for-css-import-parent-folders
May 14, 2020
Merged

CSS - Search in parent folders for node module resolution#81555
Martin Aeschlimann (aeschli) merged 3 commits into
microsoft:masterfrom
penx:feature/node-module-resolution-for-css-import-parent-folders

Conversation

@penx

@penx Alasdair McLeay (penx) commented Sep 27, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

Further to #70693 which was reworked by Martin Aeschlimann (@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 Pine (@octref) in #78894 (comment).

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

This was referenced Sep 27, 2019
Closed
Closed
@octref Pine (octref) added this to the October 2019 milestone Sep 30, 2019
@penx

Alasdair McLeay (penx) commented Jan 5, 2020

Copy link
Copy Markdown
Contributor Author

Pine (@octref) Martin Aeschlimann (@aeschli) is there anything holding up merging this PR? Any more I can do?

@penx

Alasdair McLeay (penx) commented Mar 23, 2020

Copy link
Copy Markdown
Contributor Author

Daniel Imms (@Tyriar) any more I can do to get this in to a release?

@Tyriar

Daniel Imms (Tyriar) commented Mar 23, 2020

Copy link
Copy Markdown
Contributor

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

@penx Alasdair McLeay (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 Alasdair McLeay (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

Alasdair McLeay (penx) commented Mar 23, 2020

Copy link
Copy Markdown
Contributor Author

#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 Pine (octref) modified the milestones: March 2020, April 2020 Apr 1, 2020
@aeschli

Martin Aeschlimann (aeschli) commented May 5, 2020

Copy link
Copy Markdown
Contributor

Alasdair McLeay (@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 ?

@penx
Alasdair McLeay (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

Alasdair McLeay (penx) commented May 6, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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

@aeschli
Martin Aeschlimann (aeschli) merged commit 815d94e into microsoft:master May 14, 2020
@aeschli

Martin Aeschlimann (aeschli) commented May 14, 2020

Copy link
Copy Markdown
Contributor

Thanks Alasdair McLeay (@penx)!

@penx
Alasdair McLeay (penx) deleted the feature/node-module-resolution-for-css-import-parent-folders branch May 14, 2020 15:03
@penx
Alasdair McLeay (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.

Reviewers

@aeschli Martin Aeschlimann (aeschli) Awaiting requested review from aeschli

Assignees

@aeschli Martin Aeschlimann (aeschli)

Labels

None yet

Projects

None yet

Milestone

May 2020

5 participants

@penx @Tyriar @aeschli @octref @alexdima