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

svelte.preprocess #876

Closed
Rich-Harris opened this issue Sep 28, 2017 · 10 comments
Closed

svelte.preprocess #876

Rich-Harris opened this issue Sep 28, 2017 · 10 comments

Comments

@Rich-Harris
Copy link
Member

One of the dangers of adding preprocessor support to svelte.compile is the possibility of fragmentation — if I install a component from npm and it uses SASS and CoffeeScript, I can't use the uncompiled version in my app without mirroring the component author's preprocessing step.

An idea for satisfying both needs simultaneously — a svelte.preprocess function that generates a 'standard' (i.e. plain CSS/JS) component file:

import postcss from 'svelte-postcss';
import vars from 'postcss-simple-vars';

const component = await svelte.preprocess(`
<h1>Hello {{name}}!</h1>

<style lang='postcss'>
  h1 {
    color: $somevar;
  }
</style>
`, {
  preprocessors: {
    postcss: postcss({
      plugins: [
        vars({
          somevar: 'red'
        })
      ]
    })
  }
});

assert.equal(component.toString(), `
<h1>Hello {{name}}!</h1>

<style>
  h1 {
    color: red;
  }
</style>
`);

That would work in its current form. Sourcemaps would be broken though. We could go a step further and allow separated inputs to svelte.compile, like so:

compiled = svelte.compile({
  html: {
    code: `<h1>Hello {{name}}!</h1>`,
    map: {...}
  },
  css: {
    code: `h1 { color: red; }`,
    map: {...}
  }
}, opts);

If svelte.preprocess generated output like that, and svelte.compile knew how to stitch sourcemaps together, then a) we'd retain accurate sourcemaps and b) we could even allow separated input files, which is something people have asked for.

That could happen in a second step though — as long as the output from svelte.preprocess has a toString method, it'd work with svelte.compile today.

I think I probably prefer this to changing the behaviour of svelte.compile. Need to consider the impact it would have on build tool plugins. Also, something we should bear in mind is if we want to think about sharing CSS between components via @import (with encapsulation, without duplication) to ensure we don't make any design decisions that make that difficult.

@tivac
Copy link
Contributor

tivac commented Sep 28, 2017

So in this proposal what would the default export from svelte-postcss look like? Would it get a reference to each <style> block in the component? Would it have access to rewrite the HTML?

My use-case is integrating modular-css with single-file svelte components, so that's gonna slant all my questions to that particular bent. I could of course write my own tool that can pre-process svelte components & rewrite the CSS/HTML but I'm curious how official support for that sort of a workflow might work.

@Rich-Harris
Copy link
Member Author

I'd imagined the default export would look something like this:

export default postcss(css) {
  return {
    code: '...',
    map: {...}
  };
}

Would it get a reference to each <style> block in the component?

At present, you can only have one. If we were to relax that restriction then presumably the preprocess function would be invoked for each one.

I hadn't really thought about it in terms of rewriting the markup at the same time. There's definitely a case for that. And if so, the function would need to be invoked per-component rather than per-<style>.

You could have an interface that took the entire component string as its argument and transformed it, but at that point the API really doesn't offer any convenience over just doing this:

compiled = svelte.compile(preprocess(source), {...});

or...

// rollup.config.js
export default {
  // ...
  plugins: [
    svelteModularCss(...),
    svelte(...)
  ]
};

One gotcha — dynamic classes would need to be rewritten. This...

<div class='{{foo(bar)}}'>...</div>

<style>
  .this-thing {
    color: red;
  }

  .that-thing {
    color: blue;
  }
</style>

<script>
  export default {
    helpers: {
      foo(bar) {
        return bar ? 'this-thing' : 'that-thing';
      }
    }
  };
</script>

...would have to become this:

<div class='{{__transform(foo(bar))}}'>...</div>

<style>
  :global(.this-thing-xyz123) {
    color: red;
  }

  :global(.that-thing-xyz123) {
    color: blue;
  }
</style>

<script>
  var classMap = {
    'this-thing': 'this-thing-xyz123',
    'that-thing': 'that-thing-xyz123'
  };

  function __transform(classes) {
    return classes.split(' ').map(name => classMap[name] || name).join(' ');
  }

  export default {
    helpers: {
      __transform,
      foo(bar) {
        return bar ? 'this-thing' : 'that-thing';
      }
    }
  };
</script>

That would basically involve the preprocessor reimplementing a bunch of Svelte's parsing logic though. Hmm...

@tivac
Copy link
Contributor

tivac commented Oct 4, 2017

That's a good point, my lack of familiarity with some of svelte's functionality makes me ask silly questions sometimes. It seems like the complexity level of "natively" integrating modular-css or something like it is probably prohibitive, I can continue using the rollup plugin and exposing the exported object of classnames via a helper or something.

Still, I do think that letting a preprocessor see the constituent pieces of a component sounds like a good idea. It seems like it would be more useful if it at least got the different pieces after they'd been separated out by svelte instead of just one big string. Something like { template : ..., style : ..., script ... } would lessen some of the parsing burden on preprocessor authors and ensure that it was consistent with svelte's internal handling. That would at least potentially open the door to some level of future modular-css-style preprocessing if someone were really determined.

esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 20, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 20, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 20, 2017
@esarbanis
Copy link
Contributor

I have started working on a very basic implementation for css preprocessing, but I wish I had read this issue first, as no thought was given for properly handling the sourceMaps.

It is basically a simple decorator for the compile function, that synchronously calls a user provided preprocessor function. The plus side of that is that the framework will not have dependencies to css preprocessing libraries.

I would really appreciate any feedback on this, as I would really like to spend more time on making it a proper feature.

@Rich-Harris
Copy link
Member Author

@esarbanis thanks! I think we probably want to avoid changing the behaviour of svelte.compile, and instead put that logic in svelte.preprocess — that way we can support asynchronous preprocessing without introducing a breaking change. Something like this?

export async function preprocess(source, opts) {
  if (opts.style) {
    const match = /<style([\S\s]*?)>([\S\s]*?)<\/style>/ig.exec(str);
    if (match) {
      const attributes: Record<string, string | boolean> = parseAttributes(match[1]);
      const content: string = match[2];
      const processed: { code: string, map: SourceMap | string } = await preprocessor({
        content,
        attributes
      });
      source = source.replace(styles, processed.code || content);
    }
  }

  // TODO maybe add opts.script, or even opts.component
  // for transforming markup+styles simultaneously, per
  // @tivac's use case. But that could be a future version

  return {
    // TODO return separated output, in future version where svelte.compile supports it:
    // style: { code: styleCode, map: styleMap },
    // script { code: scriptCode, map: scriptMap },
    // template { code: templateCode, map: templateMap },

    toString() {
      return source;
    }
  };
}

function parseAttributes(str: string) {
  // this won't handle edge cases, but good enough for now...
  const attrs = {};
  str.split(/\s+/).filter(Boolean).forEach(attr => {
    const [name, value] = attr.split('=');
    attrs[name] = parseAttributeValue(value);
  });
  return attrs;
}

Then you'd use it like so:

const preprocessed = await svelte.preprocess(source, {
  style: ({ content, attributes }) => {
    if (attributes.type !== 'text/scss') return null; // don't transform
    return new Promise((fulfil, reject) => {
      sass.render({
        data: content,
        includePaths: [
          path.resolve(__dirname)
        ]
      }, function (err, result) {
        if (err) reject(err);
        else fulfil({ code: result.css.toString, map: result.map });
      });
    });
  }
});

const { code, map } = svelte.compile(preprocessed);

The other advantage of separating preprocess from compile is that component authors can use whatever wacky language they want to but still distribute a component file with vanilla CSS (only shipping compiled components is bad, because it reduces Svelte's opportunities to optimise the resulting app).

What do you think?

esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 25, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 26, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 26, 2017
@esarbanis
Copy link
Contributor

I am sorry for the spam in this thread 🙁 , I made some formatting fixes that actually managed to brake the build ...

@Rich-Harris WDYT so far?

esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 26, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 27, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 27, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 29, 2017
esarbanis added a commit to esarbanis/svelte that referenced this issue Nov 29, 2017
@Rich-Harris
Copy link
Member Author

This is available in 1.44 thanks to @esarbanis. Docs on the README

@kaisermann
Copy link
Member

kaisermann commented May 7, 2018

@Rich-Harris Sorry for reviving this, but just to make sure I'm not missing anything:

If I publish a component built with, for example, scss, the developer will be forced to add the scss compile process to its preprocess.style, right?

@Rich-Harris
Copy link
Member Author

not necessarily — you could use svelte.preprocess in your build step and include the preprocessed component in your package. In fact, that's what I'd recommend.

@kaisermann
Copy link
Member

kaisermann commented May 9, 2018

In this case, you mean the whole component (script and styles), not only the css, right? I'm having trouble to wrap my head around how to include the preprocessed component (script and styles) in the svelte.preprocess step.

I was trying to include only the components preprocessed css, but the appended hashes and :global were making it impossible...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants