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 extract option #7

Closed
wants to merge 1 commit into from
Closed

Add extract option #7

wants to merge 1 commit into from

Conversation

sloanelybutsurely
Copy link

This is in response to #4.

This PR adds an additional option, extract, which is expected to be a string or false-y. When a path is provided as the value to extract, during the build, a css file will be output to that path.

To achieve this I maintain a mapping of source file name to compiled source and concatenate all compiled sources before writing to disk.

Currently this solution is very inefficient as it will write to the extract destination file as many times as there are source files. I'd imagine that this would be very slow on a large codebase.

If there are hooks for when compilation is complete in rollup, I could use that to trigger a write. I originally thought about appending to the file which would save on rewriting existing output but that solution breaks down when rollup is run in watch mode and an increment build occurs.

Other options could include throttling the disk writing but that could get rather complicated.

I'm happy to make any changes. There wasn't guide on contributing so I just followed my ❤️ .

@egoist
Copy link
Owner

egoist commented Sep 8, 2016

If there are hooks for when compilation is complete in rollup, I could use that to trigger a write.

I can't find any related plugin API for this either. Maybe some suggestions from @TrySound & @Rich-Harris 😝

@TrySound
Copy link

TrySound commented Sep 8, 2016

I guess you are lookin for something like this
rollup/rollup#806

@sloanelybutsurely
Copy link
Author

Indeed

@wesleyboar-fka-iosulfur

This seems like just what I need. Thank you!

I would like to test it, but how can I add this dependency to my package.json in a way that works? I added "rollup-plugin-postcss": "egoist/rollup-plugin-postcss#fdb7cc1", and it is installable, but the resulting install is missing the index.js that "rollup-plugin-postcss": "^0.2.0" provides. It only has a src/index.js.

code: `export default ${JSON.stringify(getExport(result.opts.from))}`,
map: { mappings: '' }
};
}
const code = `export default ${injectFnName}(${JSON.stringify(result.css)},${JSON.stringify(getExport(result.opts.from))});`;
const map = options.sourceMap && result.map

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the extract option use the sourceMap option to create a CSS source map?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can certainly give it a go!

@sloanelybutsurely
Copy link
Author

@wesleyboar This plugin is transpiled before being published; however, these built files are not committed to git. This means that if you install via git instead of npm, you're getting an unbuilt project. A surefire way to get it working locally to test out would be to clone the repo, checkout this branch, run npm link in this project's directory and npm link rollup-plugin-postcss in the project you are testing in. Once the project is linked, you can run npm run build to create the index.js file you need.

Copy link

@anilanar anilanar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you can get rid of outputMappings and replace fs.writeFileSync with fs.appendFile, all assuming transform function is called once per id, hence once per file. That would help with performance until buildend hook is introduced in rollup.

@posva
Copy link

posva commented Dec 22, 2016

@zperrault What happened? 😢

@flying-sheep
Copy link
Contributor

@zperrault why did you close this?

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

Successfully merging this pull request may close these issues.

None yet

7 participants