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

Plugin API for declaring dependencies programmatically #1203

Closed
Rich-Harris opened this issue Dec 30, 2016 · 10 comments
Closed

Plugin API for declaring dependencies programmatically #1203

Rich-Harris opened this issue Dec 30, 2016 · 10 comments

Comments

@Rich-Harris
Copy link
Contributor

Via rollup/rollup-watch#23 (cc @droooney, @tivac). Currently, the only way for a file to declare a dependency on another file is for it to be convert to JavaScript with an import statement. That means that the only way to handle things like @import statements in CSS or compile-to-CSS languages is to convert them into JavaScript somehow, which sounds a bit awkward and haphazard.

Normally this isn't a problem since, if you create a CSS file by following @import statements and gluing multiple CSS files together, the build works as expected. But if you're using rollup-watch, it means that changes to the imported files don't have any effect.

If there was a way for plugins to declare a file's dependencies programmatically – in other words, if this...

/* styles/main.css */
@import './layout.css';
@import './typography.css';
// app.js
import './styles/main.css';

...caused styles/layout.css and styles/typography.css to be registered as dependencies of styles/main.css, as well as styles/main.css being registered as a dependency of app.js due to the import declaration – that problem would be solved.

Proposed API:

export default function myPlugin ( options = {} ) {
  return {
    name: 'my-plugin',

    transform ( code, id ) {
      // some code happens...

      return {
        code: 'SOME_CODE',
        map: { version: 3, ... },
        dependencies: [
          // paths subject to usual resolution logic, so can be
          // relative or absolute, or you can supply your own
          // plugin resolve hook if needs be
          './typography.css',
          '/path/to/project/styles/layout.css'
        ]
      };
    }
  };
}

Note that those dependencies would be in addition to any discovered by JavaScript import declarations, and would not be loaded by Rollup – they would just be added to the dependency graph metadata at the end of the bundling process so that rollup-watch would be able to add file watchers.

@tivac
Copy link
Contributor

tivac commented Dec 30, 2016

This is fine, but I think documentation on returning import statements would be fine too. It took me a bit to get there but either it's not a big deal or Stockholm syndrome kicked in and I don't mind the current paradigm at all. It does exactly what you'd want and leaves no trace after treeshaking.

@nathancahill
Copy link
Contributor

I'd love to see this.

@aMarCruz
Copy link
Contributor

aMarCruz commented May 24, 2017

@Rich-Harris any progress on this?
rollup-plugin-pug inserts the "import" for included files, but this has no effect in dependency detection.

Example:

//- main.pug
include ./inner.pug
p Hello

compiles to:

import pug from "pug-runtime";
import "/foo/bar/inner.pug";
export default function( locals ) {
  // (... code that returns "<p>Hello</p>" here...)
}

inner.pug is recompiled whenever is changed, but the output goes to nowhere and main.pug remains unchanged because there's no reference to inner.pug on it (pug inline the included files).

@katrotz
Copy link

katrotz commented Jun 9, 2017

Same problem here, would love to see this API change implemented.
Did try various workarounds for the less-modules plugin without any luck.

@vladshcherbin
Copy link
Contributor

A very needed feature indeed, would help in a lot of plugins.

@kib357
Copy link

kib357 commented Mar 28, 2018

Also would love to see this. It's very uncomfortable inconvenience for projects that includes less/sass/stylus/etc with custom import system.

@tivac
Copy link
Contributor

tivac commented Jun 8, 2018

Just talked to @guybedford about this. I was originally ambivalent about this feature but now that bundle splitting and the asset emission API have landed I think something like this is very necessary.

@guybedford
Copy link
Contributor

PR created at #2259.

@tivac
Copy link
Contributor

tivac commented Aug 6, 2018

@guybedford This can be closed now, yes?

@kib357
Copy link

kib357 commented Aug 7, 2018

#2259 solved issue for me

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

9 participants