Lombiq / NodeJs-Extensions

Utilities and extensions for Node.js, used in ASP.NET (Core) MVC and Orchard (Core) CMS development.
BSD 3-Clause "New" or "Revised" License
7 stars 2 forks source link

NodeJsExtensionsGlobalESLintConfigurationDirectory and NodeJsExtensionsGlobalStylelintConfigurationDirectory only works with a submodule (OSOE-611) #44

Closed Piedone closed 1 year ago

Piedone commented 1 year ago

Adding the following to the Directory.Build.props file in a solution where a project uses NE from NuGet...

  <PropertyGroup>
    <!-- Instruct Node.js Extensions to create .eslintrc and .stylelintrc files in the current directory, which is the
         solution root directory. -->
    <NodeJsExtensionsGlobalESLintConfigurationDirectory>$(MSBuildThisFileDirectory)</NodeJsExtensionsGlobalESLintConfigurationDirectory>
    <NodeJsExtensionsGlobalStylelintConfigurationDirectory>$(MSBuildThisFileDirectory)</NodeJsExtensionsGlobalStylelintConfigurationDirectory>
  </PropertyGroup>

...yields build errors, and e.g. the following during SCSS build:

no-path(1,1): error ERROR: No configuration provided for ...\site.scss.

The NuGet sample actually indeed does the opposite, and local config files work.

I have a faint memory of this being an acceptable disadvantage that we couldn't somehow work around. However, I'd expect this to be documented under the docs pages where the above configs are mentioned.

Jira issue

Piedone commented 1 year ago

@0liver is this an omission in the docs or a bug in the code?

0liver commented 1 year ago

I'll (have to) take a look.

0liver commented 1 year ago

At the current stage, I see three options to handle this:

  1. Add some pieces of documentation to reflect the current situation.
  2. Do 1. plus add a Warning from code when we detect this situation.
  3. Implement the missing functionality. This is now possible thanks to the ExecWithMutex task.

I would personally vote for 2. and delay implementing 3. to a point where it will be needed.

Piedone commented 1 year ago

Documentation will be enough, for now.