alexlafroscia / ember-cli-stencil

Automatic discovery of Stencil.js components for your Ember application
26 stars 7 forks source link

Not compatible with Stencil compiler >= 0.9.1 #5

Closed rbrcurtis closed 6 years ago

rbrcurtis commented 6 years ago

The latest version of stencil suggests using module and main props in package.json instead of browser. This lib (which I love, btw), looks for the browser prop. The lack of that is breaking this.

rbrcurtis commented 6 years ago

stencil 0.9.1 gives this warning when building:

[ WARN  ]  build warn
           package.json "browser" property is set to "dist/toastcomponent.js". However, for maximum compatibility with
           all bundlers it's recommended to not set the "browser" property and instead ensure both "module" and "main"
           properties are set.
alexlafroscia commented 6 years ago

Interesting -- thanks for catching this! Hopefully there'e still some means for discovering the right file to import into the Ember build; my guess is that main is for a SSR environment and module is for a component being imported as ES6 modules, which would be nice if Ember supported lazy-loading through import() the way that Webpack and all do.

alexlafroscia commented 6 years ago

I've started to investigate this and unfortunately, without the browser field, it's going to be a lot harder for me to detect where the files live and to pull them in without the user configuring anything.

Previously, the browser field pointed right to the files that the Ember application needs.

Now, we have a pointer to a bunch of ESM modules that we're unable to consume from the Ember CLI. While these files are fine for tools like Webpack or Rollup, the Ember CLI can't handle the dynamic import() statements that allow the components to be lazy-loaded. We require the original browser file.

The name and location does follow a pattern, __package_root__/dist/__namespace__.js, but the problem is that I have no way to guess namespace correctly because the stencil.config.js file is not part of the published package.

I have started a thread on Slack to try to ask why browser was removed and maybe see if we can re-introduce it. I don't understand why it would be negatively impactful to have it there, but I'm ready to either learn a good reason or try to lobby for them to introduce it again.

A link to the Slack thread: https://stencil-worldwide.slack.com/archives/C789G3X1R/p1530898958000043

rbrcurtis commented 6 years ago

I don't see any problem with you requiring that people have an additional prop in their package.json to read from for this.

alexlafroscia commented 6 years ago

A prop in their Stencil component package.json?

Ideally, they would just maintain the original browser property that points to the same file that it does right now.

The problem is that the Stencil compiler is actively discouraging people from doing this -- which I don't quite understand. It seems they're concerned about tools consuming the package and getting confused by a main, module and browser field but I don't see what the harm in that is.

It's really too bad that Ember apps can't consume import() yet -- I think that the packager work being done right now should solve this, since we could be able to leverage Rollup under the hood, but I don't like the idea of this addon requiring a specific packager and I have no idea when that work will land.

alexlafroscia commented 6 years ago

Some good news!

ember-auto-import supports both ES modules and dynamic import! I will investigate whether I can leverage that to import the files instead, which seems to be the direction that Stencil is leaning toward if they won’t expose a pointer to the non-ESM bundle.

alexlafroscia commented 6 years ago

Working on this with some success! However, there seems to be a bug in the latest @stencil/core

https://github.com/ionic-team/stencil/issues/930

alexlafroscia commented 6 years ago

This should be working now -- make sure to upgrade to @stencil/core@0.12.4 or newer. The build that that version of the compiler produces is actually valid to be consumed as ESM (they were producing an invalid build for a while) and ember-auto-import is leveraged now to import that package.

These changes are available in 0.4.0 which I just released.

rbrcurtis commented 6 years ago

Very cool. Thank you for taking care of this.

alexlafroscia commented 6 years ago

Sure thing. Please let me know as you see other issues!