stencil icon indicating copy to clipboard operation
stencil copied to clipboard

Incorrect context provided for bundler plugin transforms

Open DesignByOnyx opened this issue 5 years ago • 9 comments

Stencil version:

 @stencil/[email protected]

I'm submitting a:

[x] bug report [ ] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior: According to the Stencil docs the stencil bundler uses rollup under the hood and we should be able to provide our own rollup plugins. However, the "transform" hook is not called with the proper rollup plugin context. This is not only breaking the ability for me to create my own plugin, but it also breaks community plugins which reference this.xxxx inside the transform method.

Expected behavior: I would expect that my rollup plugin transform hook be called with the correct rollup plugin context. See the rollup plugin context docs.

Steps to reproduce: You can use the existing @stencil/postcss plugin and try calling this.addWatchFile() from inside the transform function.

Other information: I am willing and eager to fix this if you can provide some insight. I have been digging through the compiler code for hours and cannot figure out the abstraction you have created around rollup. The existing buildCtx and compilerCtx do not contain the rollup plugin context, and I cannot figure out how to inject it.

UPDATE: the primary issue I am running into is the fact that @import'd files are not being watched (using either @stencil/postcss or @stencil/sass). This could be easily fixed if we could call this.addWatchFile from inside the transform function.

Update 2022.04.04

The following information is from #3303:

Steps to Reproduce

See the linked fork: https://github.com/ionic-team/stencil-sass/compare/master...DesignByOnyx:rwheale/1707-rollup-context-bug?expand=1

To get you there quicker:

git clone https://github.com/DesignByOnyx/stencil-sass.git
cd stencil-sass
npm i
npm run build
npm test

Code Reproduction URL

https://github.com/ionic-team/stencil-sass/compare/master...DesignByOnyx:rwheale/1707-rollup-context-bug?expand=1

DesignByOnyx avatar Jul 03 '19 18:07 DesignByOnyx

@stencil/postcss is not a rollup plugin, and it's not intended to be one.

manucorporat avatar Jul 07 '19 12:07 manucorporat

  • @stencil/postcss is listed on the Related Plugins section of the docs which describe building custom rollup plugins: https://stenciljs.com/docs/plugins
  • It also implements a transform function which accepts the code and id arguments and returns an object with the code property - so it quacks rather loudly like a rollup plugin.
  • Most importantly, the @stencil/postcss plugin is configured the same way you would any other rollup plugin in the plugins section of the stencil config: https://stenciljs.com/docs/module-bundling

This issue isn't about @stencil/postcss, but rather building a custom rollup plugin and having the transform hook called with the proper plugin context. I was using an existing plugin as an example so you don't have to write one from scratch. The same would apply if you wrote your own rollup plugin from scratch or used a community-maintained rollup plugin and expect this to be what it should.

DesignByOnyx avatar Jul 08 '19 16:07 DesignByOnyx

Any updates on this? I'm trying to write a Rollup plugin and am not able to call this.addWatchFile since it always seem to be undefined.

jgroth avatar Feb 12 '20 08:02 jgroth

Hey there, thank you for the patience getting back to you. The new team is getting started and we're working through the backlog now.

I'm going to label this and sort it so we can better understand how to improve Rollup for folks. If you have had better luck since you originally posted, could you share your progress with me here? Even adding a reproduction case would be awesome.

splitinfinities avatar Aug 03 '21 19:08 splitinfinities

Thanks for the issue! This issue is being closed due to inactivity. If this is still an issue with the latest version of Stencil, please create a new issue and ensure the template is fully filled out.

Thank you for using Stencil!

ionitron-bot[bot] avatar Sep 02 '21 20:09 ionitron-bot[bot]

Hey ionitron-bot, we didn't mean to close this. Bad bot!

rwaskiewicz avatar Sep 03 '21 12:09 rwaskiewicz

Hey folks,

We're working through revamping our labeling system/triaging in Stencil. If anyone interested in helping us narrow down this issue, could you please open a reproduction case to this issue and comment here? That'll keep this issue going/possible for the team to look into. Otherwise, it may be closed due to inactivity. Thanks!

rwaskiewicz avatar Mar 25 '22 14:03 rwaskiewicz

Thanks @DesignByOnyx, I've pulled down your repro (from #3303) and can verify this is erroring. I've labelled this issue for the team to take a closer look at. Thanks again!

rwaskiewicz avatar Apr 04 '22 12:04 rwaskiewicz

The issue is locked, so I can't reply.

I'm not in a place to test, but you can replicate this by using any of the existing rollup plugins currently maintained by your team: @.***/postcss, ***@***.***/sass. Simply try to call this.addWatchFile (or any of the context methods) inside the plugin.

On Fri, Mar 25, 2022 at 8:45 AM Ryan Waskiewicz @.***> wrote:

Hey folks,

We're working through revamping our labeling system/triaging in Stencil. If anyone interested in helping us narrow down this issue, could you please open a reproduction case to this issue and comment here? That'll keep this issue going/possible for the team to look into. Otherwise, it may be closed due to inactivity. Thanks!

— Reply to this email directly, view it on GitHub https://github.com/ionic-team/stencil/issues/1707#issuecomment-1079098018, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5P6EGRATQHRIQZ4CTO4LVBXGRPANCNFSM4H5ITZLQ . You are receiving this because you authored the thread.Message ID: @.***>

DesignByOnyx avatar Oct 11 '22 08:10 DesignByOnyx