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

Watch partials #29

Closed
erezmus opened this issue Feb 16, 2017 · 27 comments
Closed

Watch partials #29

erezmus opened this issue Feb 16, 2017 · 27 comments

Comments

@erezmus
Copy link

erezmus commented Feb 16, 2017

Hi,

I'm currently using the plugin to generate my css from an scss file imported in the main js file as such:

import './../styles/main.scss'

Running the rollup command with the watch flag, when i save the main.scss files it recompiles fine, however if I change any of the partials, its not detected.

Is there a way for the rollup watch to extend to those partials?

@differui
Copy link
Contributor

differui commented Feb 17, 2017

Hi @mwerezi , I think --watch flag only helps recompile automatically. so u have to refresh browser manually. Maybe u need live reload during development.Checkout quick start docs from rollup wiki.

I use below configuration file and auto recompile works:

// package.json
{
  "devDependencies": {
    "rollup": "^0.41.4",
    "rollup-plugin-babel": "^2.7.1",
    "rollup-plugin-sass": "^0.4.9",
    "rollup-watch": "^3.2.2"
  }
}
// rollup.config.js
import babel from 'rollup-plugin-babel'
import sass from 'rollup-plugin-sass'

export default {
    entry: './src/index.js',
    dest: './dist/index.js',
    format: 'cjs',
    plugins: [
        sass({
            insert: true
        }),
        babel({
            runtimeHelpers: true
        })
    ]
}
> ./node_modules/.bin/rollup -c --watch
bundling...
bundled in 318ms. Watching for changes...
bundling...
bundled in 59ms. Watching for changes...
bundling...
bundled in 33ms. Watching for changes...

@ngryman
Copy link

ngryman commented Mar 9, 2017

I think what @mwerezi is saying is that rollup is not aware of the sass dependency tree and does not recompile sass dependencies but only the entry file (i.e. main.scss).

Let's say my main.scss imports (with sass imports) a file some/partial.scss:

@import 'some/partial.scss';

If I modify main.scss it will be detected by rollup as it's in the dependency tree via an es6 import. But if I modify some/partial.scss, it won't as rollup has no idea of the sass dependency tree.

@mwerezi One workaround is to import everything with es6 imports and not use sass @import at all.

@differui Still it feels we are losing something here. I don't really know what facilities rollup offers to plugin authors, but do you think there would be a way to make rollup aware of those sass @imports?

@differui
Copy link
Contributor

differui commented Mar 9, 2017

@mwerezi Oops, sorry for misunderstood.

We use node-sass compile .scss and .sass in this plugin.So it's not possible that rollup can track dependency tree of stylesheets for us.

rollup-watch looks like using modules entry to track dependencies.As we can see this plugin leave this entry empty.

// ./src/index.scss
@import './partial.scss';

body {
    font-size: 12px;
    color: green;
}
// ./src/index.js
import css from './index.scss';

console.log(css);
// build.js
const rollup = require('rollup');
const sass = require('rollup-plugin-sass');
const babel = require('rollup-plugin-babel');

rollup.rollup({
    entry: './src/index.js',
    dest: './dist/index.js',
    format: 'cjs',
    plugins: [
        sass({
            insert: true
        }),
        babel({
            runtimeHelpers: true
        })
    ]
}).then(bundle => {
    console.log(bundle.modules);
}).catch(e => {
    console.log(e);
});

And here is the output:

[
  {
    id: ~/src/index.scss',

    // rollup-watch track nothing
    dependencies: [], 
    code: 'export default ___$insertStyle("body {\\n  font-size: 12px; }\\n\\nbody {\\n  color: green; }\\n");;',
    originalCode: '@import \'./partial.scss\';\n\nbody {\n    color: green;\n}\n',
    ast:
     { type: 'Program',
       start: 0,
       end: 95,
       body: [Object],
       sourceType: 'module' },
    sourceMapChain: [ [Object], [Object] ],
    resolvedIds: {}
  },
  {
    id: '~/src/index.js',

    // rollup-watch track index.scss
    dependencies: [ '/Users/differui/Sandbox/rollup-watch/src/index.scss' ],
    code: 'import css from \'./index.scss\';\n\nconsole.log(css);',
    originalCode: 'import css from \'./index.scss\';\n\nconsole.log(css);\n',
    ast:
     { type: 'Program',
       start: 0,
       end: 50,
       body: [Object],
       sourceType: 'module' },
    sourceMapChain: [ [Object] ],
    resolvedIds: { './index.scss': '~/src/index.scss' } 
  }
]

I don't know how other plugins put those stuff in modules (maybe include resolvedIds) for now. But take a step forward on workaround that @mwerezi talked.This plugin takes over the dirty work that transpile @import ./path/to/style.scss to import ./path/to/style.scss as part of the output.As so the tracking maybe works.

@ngryman
Copy link

ngryman commented Mar 9, 2017

@differui I think it's potentially possible to inject sass dependencies into rollup as others plugins do it in some way. commonjs would be the most famous. It parses the ast and search for require and then somehow (transpilation?) make rollup aware of it.

So theoretically I guess, it would be possible to transpile or grab every @import in the sass source and make rollup aware of those dependencies. How to do that, I don't know 🤓

@ngryman
Copy link

ngryman commented Mar 9, 2017

@Rich-Harris sempai, do you think it would be something achievable?

Context

@Rich-Harris
Copy link

This is something that we'd like to implement: rollup/rollup#1203

Does the proposal in that issue (returning a { code, map, dependencies } object, or a promise that resolves to one) seem like it would work? I'm not familiar with how the SASS compiler works, but presumably it has a way of reporting which files were imported?

@ngryman
Copy link

ngryman commented Mar 9, 2017

@Rich-Harris Thanks for your quick feedback 🎉
Given my limited knowledge of rollup plugin API I feel that rollup/rollup#1203 would allow to resolve this issue.

@differui What do you think?

@differui
Copy link
Contributor

differui commented Mar 10, 2017

@Rich-Harris It's fine to have the mechanism that plugin tells rollup dependencies information.
@mwerezi I did little doc research.node-sass flatten dependency tree into a plain array which stored as includedFiles in result-object.

// sample.scss
@import "partical-a.scss";
@import "partical-b.scss";

body {
    color: red;
}
// partical-a.scss
@import "./partical-c.scss";

body {
    font-size: 12;
}
// partical-b.scss
body {
    font-style: italic;
}
// partical-c.scss
body {
    background-color: white;
}
// compile.js
const fs = require('fs');
const sass = require('node-sass');

sass.render({
    data: fs.readFileSync('./sample.scss').toString()
}, (err, result) => {
    if (err) console.error(err);
    else console.log(result.stats.includedFiles);
});

Here is the output:

[ 
  '/absolut/path/to/partical-a.scss',
  '/absolut/path/to/partical-b.scss',
  '/absolut/path/to/partical-c.scss'
]

differui pushed a commit that referenced this issue Mar 10, 2017
differui pushed a commit that referenced this issue Mar 10, 2017
@differui differui mentioned this issue Mar 10, 2017
@erezmus
Copy link
Author

erezmus commented Mar 16, 2017

@ngryman If I remember well, I tried that but then I had a problem with rollup-plugin-hash although i've now switched to doing my own hashing. I might give es6import a go again

@pawel-kontakt
Copy link

@ngryman You've propose workaround:

One workaround is to import everything with es6 imports and not use sass @import at all.
I've tried it but sass is loosing context when files are setup in js file.

For example if I have Styles.js with:
import 'scss/variables.scss; import 'scss/components.scss

Variables defined in variables.scss are not visible in components.scss. Do you have idea how to workaround this?

@differui
Copy link
Contributor

differui commented Apr 18, 2017

@pawel-kontakt I think scss/variables.scss should be imported both Styles.js and scss/components.scss. One for deps tracking in rollupjs and another for keeping context in sass.

@ngryman
Copy link

ngryman commented Apr 20, 2017

@pawel-kontakt You have to use sass @import for sass related stuff (variables, mixins, ...). You only use JavaScript import for outputed CSS.

scss/components.scss

@import 'variables';

// ...

index.js

import './scss/components.scss';

@differui No need to import scss/variables.scss in JavaScript realm as it does not output anything.

@differui
Copy link
Contributor

@ngryman If scss/variables.scss not imported in JavaScript realm, rollup.js can not trace it and not rebuild after scss/variables.scss changed.Cause we lack the mechanism that plugin tells rollup.js dependencies information.

@pawel-kontakt
Copy link

I was afraid that including same file in js and scsss will cause it content to be duplicated in output - but it is not - I've tested it in simple config.

For me this solution is good enough, thank you.

@differui
Copy link
Contributor

@pawel-kontakt It's OK.rollup.js will eliminate unused variables.

@ngryman
Copy link

ngryman commented Apr 21, 2017

@differui Yes true forgot about that.

@katrotz
Copy link

katrotz commented Apr 25, 2017

I solved it for the less plugin by declaring the less imports as ES module dependencies (as @differui
suggested). Now rollup considers these files as dependencies and watches them too.

@nathancahill
Copy link
Contributor

I solved @pawel-kontakt's issue in a new Sass plugin: http://github.com/nathancahill/rollup-plugin-collect-sass

It does a first pass to collect all of the imports, then compiles all of the imported Sass in a single context, so variables are available across JS imports, without risking CSS duplication (Rollup handles JS duplication but not CSS).

@hultberg
Copy link

Is there any solution to this issue? Looks like the MR #30 would fix it, but it needs an update

@sormy
Copy link

sormy commented Oct 26, 2018

https://github.com/es128/progeny could be integrated to recursively find all dependencies

@kazzkiq
Copy link

kazzkiq commented Oct 4, 2019

Any updates on the issue?

@Wolfr
Copy link

Wolfr commented Feb 24, 2020

I have a similar problem with rollup-plugin-scss. I was considering switching to this project, but defining SCSS partials in an .js file is not an acceptable solution to me. Will look into this more.

@sidati
Copy link

sidati commented Jun 10, 2020

@kazzkiq, rollup-plugin-postcss support partials

@kazzkiq
Copy link

kazzkiq commented Jun 10, 2020

As @sidati said, the PostCSS plugin works with partials. And PostCSS supports SASS as a plugin.

So perhaps using PostCSS may be a fit substitute for this project:

  1. Remove this plugin from your project;
  2. Add rollup-plugin-postcss instead;
  3. Add SASS plugin in your rollup.config.js;

@moritzebeling
Copy link

I don’t get it. Isn’t partials the most important thing about sass? What is a sass plugin without partials?

@JanTvrdik
Copy link

JanTvrdik commented Nov 23, 2020

This is what works for me if that helps

const mySass = (options) => ({
	name: 'my-sass',
	async load(id) {
		if (!id.endsWith('.sass')) {
			return
		}

		const sassResult = sass.renderSync({ file: id, indentedSyntax: true })
		sassResult.stats.includedFiles.forEach(it => this.addWatchFile(it))
		
		const referenceId = this.emitFile({
			type: 'asset',
			name: path.basename(id, '.sass') + '.css',
			source: sassResult.css,
		})

		return `export default import.meta.ROLLUP_FILE_URL_${referenceId}`;
	},
})

@elycruz elycruz self-assigned this Oct 7, 2023
@elycruz elycruz removed their assignment Oct 7, 2023
elycruz added a commit that referenced this issue Apr 17, 2024
issue-#29 - Added test for rollup watch feature
@elycruz
Copy link
Owner

elycruz commented Apr 17, 2024

Tests written (#129 ) - This is currently fixed in library versions 1.12.22+.

@elycruz elycruz closed this as completed Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests