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

Add a third argument to the preprocessor option that gives us the file name #571

Closed
soluml opened this issue Apr 9, 2020 · 7 comments · Fixed by #825
Closed

Add a third argument to the preprocessor option that gives us the file name #571

soluml opened this issue Apr 9, 2020 · 7 comments · Fixed by #825
Labels
bundler: webpack 📦 Issue is related to webpack bundler enhancement: approved ✅ Improvement of the current behaviour that has been approved status: has PR 😍 Issue has opened PR

Comments

@soluml
Copy link
Contributor

soluml commented Apr 9, 2020

Describe the enhancement

In the webpack linaria/loader, we have a preprocessor option which gives us the ability to further augment the text before passing it down the line. It would be very helpful to have a 3rd argument which features the output file name for this file.

Motivation

This would allow us to conditionally make changes, like, adding a Sass @use at the top of the file to include mixins and or variables.

Currently, we have to work around it doing something like so:

{
  loader: 'linaria/loader',
  options: {
    extension: '.linaria.scss',
    preprocessor: (selector, cssText) => `
      @import 'Styles/helpers';
      ${selector} { ${cssText} }
    `,
    sourceMap: stage.startsWith('develop'),
  },
}

This works, but results in importing the file over and over again a lot. With a third argument, we could determine which file we're in and conditionally add @use/@import's as needed.

@soluml soluml added the enhancement: proposal 💬 Improvement of current behaviour that needs to be discussed label Apr 9, 2020
@jayu
Copy link
Contributor

jayu commented Apr 9, 2020

@soluml As I understand, Inside preprocessor callback you want to access the filename from where styles were extracted? Or rather the path. So the use case might look like

 preprocessor: (selector, cssText, filePath) => filePath === 'foo' ? `
      @import 'Styles/helpers';
      ${selector} { ${cssText} }
    ` : 'sorry, co css',

@soluml
Copy link
Contributor Author

soluml commented Apr 9, 2020

Yea, exactly. Sorry I wasn't clear. Below is something I'd do if this was implemented:

// Somewhere in the file
const filePathMap = new Map();

// Loader Options
preprocessor: (selector, cssText, filePath) => {
  const imports = filePathMap.get(filePath) ? "" : "@use 'Styles/helpers';";  
  filePathMap.set(filePath, true);
  return `${imports} ${selector} { ${cssText} }`;
}

Basically, if this file has been written to already, don't add Sass's @use as it must be at the top of each file.

@soluml
Copy link
Contributor Author

soluml commented Apr 9, 2020

I opened up a PR for this issue: #572

@satya164
Copy link
Member

satya164 commented Apr 9, 2020

@soluml not sure if keeping external map is something you should do. files can be rebuilt at a future point. your external cache won't be consistent with the actual output.

@soluml
Copy link
Contributor Author

soluml commented Apr 9, 2020

My thought was to add the filePathMap at the top of my webpack.config.js file. The theory being that it would re-initialize anytime the build runs or re-runs so I could re-apply the "banner" text to every file. Though, in a watch situation, I'm not entirely sure if it'd be re-initialized or not... might depend on the bundler setup.

Will have to do some digging 👍

@soluml
Copy link
Contributor Author

soluml commented Apr 9, 2020

In my specific testing instance, I'm using Gatsby which leverages Webpack Dev Server. It provides hooks where I could reset the Map as needed to ensure I was setting the @use directive appropriately for each file, only once.

Not sure if that will help anyone googling for an answer to something like this, but there ya go.

@satya164
Copy link
Member

satya164 commented Apr 9, 2020

I think the easiest way would be to write a simple webpack loader which runs for CSS files, then inserts what you need into those files. With a loader, you can make sure that it runs for each file and not each rule.

@jayu jayu added bundler: webpack 📦 Issue is related to webpack bundler enhancement: approved ✅ Improvement of the current behaviour that has been approved status: has PR 😍 Issue has opened PR and removed enhancement: proposal 💬 Improvement of current behaviour that needs to be discussed labels Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bundler: webpack 📦 Issue is related to webpack bundler enhancement: approved ✅ Improvement of the current behaviour that has been approved status: has PR 😍 Issue has opened PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants