ghost
commented
1 year ago Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.
Open StfBauer opened 1 year ago
ghost
commented
1 year ago Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.
stevebeauge
commented
1 year ago Also notices that scss files from libs in monorepo are excluded from webpack loader.
My setup (working with 1.16.1, not with 1.17.1):
There's no bundling in my package library. Only a tsc invocation to build the ts/tsx files, and a copy-file action to have the scss files alongside the built js. The modules has @import (tried also with @use) declaration to reference fluentui
When I try to bundle the spfx project, which reference the lib, I got errors:
C:/path/to/worspace/packages/somelib/lib/component/somecomponent.module.scss 1:0
Module parse failed: Unexpected character '@' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
> @use "~@fluentui/react/dist/sass/References";Can provide a repro if needed
StfBauer
commented
1 year ago @stevebeauge, thanks for the confirmation. The intermediate workaround I found was to copy the node_modules files into the SPFx project via a post-install script.
// package.json or workspace
{
// ...
"scripts": {
"postinstall": "npm shx node_modules <project-folder>/node_modules"
}
// ...
}At least this way, it works.
As I mentioned before, the tilda should not be available in the SASS file because webpack is not involved until the CSS file is compiled into the 'lib' folder.
stevebeauge
commented
1 year ago @StfBauer : I don't think my issue is related to the tilda character. Actually, any @import or @use fails. The error let me think the scss files are not loaded using the proper loader.
My setup of libraries has no bundling at all. I just have es6 javascript files with import statement targeting scss files.
When I compare the webpack conf from 1.16.1 to 1.17.1 I saw a lot of changes related to loaders:
And I suppose the new loaders have different config that prevent my sass files from packages to be loaded
StfBauer
commented
1 year ago @stevebeauge SASS does not matter in the loader anymore.
SPFx compiles style the following way:
The ~ only is a Microsoft built-in shortcut path to node_modules
stevebeauge
commented
1 year ago @StfBauer : I'll create a repro and submit a new issue to keep your original issue the main subject of this issue.
StfBauer
commented
1 year ago After a bit of research, I found the malfunctioning code. It is rooted in the package @microsoft/gulp-core-build-sass, specifically in the file SassTask.js.
SassTask.js - Line: 70 onwards
return node_core_library_1.LegacyAdapters.convertCallbackToPromise(sass.render, {
file: filePath,
importer: (url) => ({ file: this._patchSassUrl(url) }),
sourceMap: this.taskConfig.dropCssFiles,
sourceMapContents: true,
omitSourceMapUrl: true,
outFile: cssOutputPath,
quietDeps: !!this.taskConfig.quietDeps
})
.catch((error) => {
this.fileError(filePath, error.line, error.column, error.name, error.message);
throw new Error(error.message);
})The options passed to the SASS render are incomplete add miss the loadPaths option, which allows importing of SASS files from different locations such as the node_modules.
The passed in SASS Option object currently looks like this:
{
file: filePath,
importer: (url) => ({ file: this._patchSassUrl(url) }),
sourceMap: this.taskConfig.dropCssFiles,
sourceMapContents: true,
omitSourceMapUrl: true,
outFile: cssOutputPath,
quietDeps: !!this.taskConfig.quietDeps
}There are multiple issues in this config:
Issue 1 - importer
The importer was required because the path to the module could not be found, which was a result of the malfunctioning configuration. There is also another issue in the this._patchSassUrl(url).
_patchSassUrl(url) {
if (url[0] === '~') {
url = 'node_modules/' + url.substr(1);
}
else if (url === 'stdin') {
url = '';
}
return url;
}As I assumed in the initial configuration, the Tilda only was SPFx specific and had no impact on the webpack composition. It was merely introduced in SASS files because webpack has a similar construct. The code above assumes a 'node_modules' folder. It neither checks if it exists nor uses the node_modules resolution of NodeJS.
Solution to Issue 1:
The code can be removed, and the configuration can get adjusted to:
{
file: filePath,
sourceMap: this.taskConfig.dropCssFiles,
sourceMapContents: true,
omitSourceMapUrl: true,
outFile: cssOutputPath,
quietDeps: !!this.taskConfig.quietDeps
}With this modification, the Tilda as a node_modules reference does not work anymore, but It has worked correctly in the first place since version 1.0.0. It was more of a lucky coincidence, hoping that the node_modules folder was there instead of checking the existence of the node_modules folder.
Issue 2 - includePath / loadPath
SASS JavaScript SDK has a built-in option to includePaths. The build of a new CSS file should resolve. The correct paths can be directly injected into the configuration.
require.main.paths - do not resolve correctly because it will only resolve the filepath of the process.
module.paths - instead used the correct node_modules path resolution based on the current @microsoft/gulp-core-build-sass module.path - returns the potential node_modules path under the current folder.
Solution to Issue 2 - includePath / loadPath
{
file: filePath,
sourceMap: this.taskConfig.dropCssFiles,
sourceMapContents: true,
omitSourceMapUrl: true,
outFile: cssOutputPath,
includePaths: module.paths, // << Added module.paths resolution
quietDeps: !!this.taskConfig.quietDeps
}These two simple steps should solve the issue for now and in future.
return node_core_library_1.LegacyAdapters.convertCallbackToPromise(sass.render, {sass.render is deprecated and will be removed in future releases.
A newer option is sass.compile and especially sass.compileSync, which is, according to the documentation, the fastest option to get sass compiled.
After this issue was resolved, the typescript compiler shows similar problems too, but worth to open another case.
stevebeauge
commented
5 months ago I wonder if the latest 1.19 release, which moved to webpack 5, solve this issue.
StfBauer
commented
5 months ago @stevebeauge Nope got worse because it is not really realated to webpack - SASS and CSS Modules comes before the webpack.
The overall build chain configuration is a bit strange in this regards.
Target SharePoint environment
SharePoint Online
What SharePoint development model, framework, SDK or API is this about?
💥 SharePoint Framework
Developer environment
None
What browser(s) / client(s) have you tested
Additional environment details
Describe the bug / error
SPFx has a multi-problem issue with the rush stack and some of the implemented components.
SASS which is now using sass-dart have some issues on how it handles CSS files in general.
Issue 1: tilda use in SASS files
The
~before the npm package is wrong because it is only required for direct use of style loader in webpack. In the case of SPFx, all SASS files get compiled into a standalone CSS. Therefore it should not be necessary. Webpack is not involved in loading the SASS file. Instead, the CSS gets picked up automatically from thelibfolder.Issue 2: @import deprecation
With the release of SASS Module System, the
@importstatement got deprecated by 1 October 2021. In July 2022, it was decided that its support should last longer.In future, this should not be in any new web part project.
Issue 3:
With the deprecations of
@importin SASS files, the correct way to write the same statement shown in issue 1 should get changed to from:To this:
This causes the following problems:
~is incorrect to use in a SASS file, and SASS does not know how to resolve it. It's a webpack feature, but before the involvement of webpack, the style gets generated into thelibfolder, which gets picked up by a standard web part css-loader.One might change tend to change the loading path to:
Without the Tilda, SASS cannot result in the node_modules folder too.
Also, do not work because the rush-stack sass task needs to know how to resolve from the project's current location of the node_modules folder.
To solve it, one can do the following.
A path to the represented node_modules folder works, but more on that in issues 4 and 5.
Issue 4: Finding the Module
While working with other NextJS, ReactJS and even custom setups, we project there need to be greater loading styles and modules resolution.
In this regard, SPFx is special, indicating a general rush-stack issue when handling SASS files.
The documentation on the module system in SASS states the following on finding the module.
Issue 5: Npm / Yarn workspace - aka Monorepo support
Yarn, as well as npm, have meanwhile an out-of-the-box support for mono repos. However, this base structural change is not supported by SPFx, at least regarding Sass Loading.
I created a sample project that demonstrates what this looks like.
SPFx
In YARN/NPM workspace, none of the modules gets installed inside the project. Instead, the live one level higher outside of the SPFx project.
Typescript can handle this scenario well, but random errors are also attached. Mostly finding the tsconfig file.
Issue 5a: Cannot find existing CSS
Let's not involve the new way of
sass:metainto style loading into this a simple load of:This result in the following error message:
Issue 5b: Cannot find existing CSS
This result in the following error message:
Issue 5c: Cannot find existing CSS with adjusted CSS link
This result in the following error message:
Issue 6: rush-stack as an option for mono-repos
Developers could subscribe to rust-stack, but the level of abstraction from common project configurations and the deep dependencies, are less safe and future-proof as a solution.
Yarn/npm workspaces keep it plain and simple and work properly together with around 99% of npm packages in the ecosystem.
Steps to reproduce
Checkout project and readme - https://github.com/StfBauer/spfx-workspace
Expected behavior
Proper implementation of SASS. Proper support for standards such as npm or yarn components.