Avoid resolving imports relative to working directory for compileString #124
Conversation
| const importer = new proto.InboundMessage.CompileRequest.Importer(); | ||
| importer.setPath(p.resolve('.')); | ||
| input.setImporter(importer); |
There was a problem hiding this comment.
@nex3 With new spec, null URL will end up with noOpImporter on embedded compiler, but legacy API wants a FilesystemImporter even without filename/url, so explicitly set it should give the compiler the correct importer.
| url: options.file ? pathToFileURL(options.file) : undefined, | ||
| url: options.file | ||
| ? pathToFileURL(options.file) | ||
| : new URL(legacyImporterProtocol), |
There was a problem hiding this comment.
Legacy API needs to have a FilesystemImporter('.') as if it has an url even if it does not. Set a special value here to let the new API know that.
| @@ -87,10 +90,19 @@ function newCompileStringRequest( | |||
| input.setSource(source); | |||
| input.setSyntax(utils.protofySyntax(options?.syntax ?? 'scss')); | |||
|
|
|||
| if (options?.url) input.setUrl(options.url.toString()); | |||
| const url = options?.url?.toString(); | |||
| if (url && url !== legacyImporterProtocol) { | |||
There was a problem hiding this comment.
The special value legacyImporterProtocol means this is from Legacy API that even if it does not set a url it should behave as if it has one down below when setting the importer.
|
Unless I'm mistaken, updating sass/dart-sass-embedded#83 so that the compiler uses a filesystem importer in the right circumstances should mitigate the need for this PR. However, it would be good to also add a JS API spec to sass-spec that verifies that CWD-relative imports don't work with |
|
Looks like this can be simplified a bit by constructing proto importer inside the legacy compile. |
|
Let me close this and open an new PR. |
This PR fixes sass/dart-sass-embedded#82.