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

[bundler] Bundled shell does not re-export ids used by multiple fragments; './x-app.js' does not provide an export named 'directive' #721

Closed
keanulee opened this issue Sep 24, 2018 · 8 comments
Assignees

Comments

@keanulee
Copy link
Contributor

Setup:

  • Shell - my-app.js
  • Multiple fragments - my-view1.js and my-view2.js

If my-view1 uses a directive (e.g. repeat):

import { repeat } from 'lit-html/directives/repeat.js';

And my-view2 uses another directive (e.g. unsafeHTML):

import { unsafeHTML } from 'lit-html/directives/unsafe-html.js';

Then both fragments will try to import directive from my-app, but my-app doesn't export it.

Repro:

$ git clone -b lit-element https://github.com/Polymer/shop.git
$ cd shop
$ npm i
$ polymer build
$ polymer serve build/esm-bundled

Navigate to the detail view (e.g. http://localhost:8081/detail/ladies_outerwear/Ladies+Pullover+L+S+Hood). Note the console error

shop-app.js:483 Uncaught (in promise) SyntaxError: The requested module './shop-app.js' does not provide an export named 'directive'
@LarsDenBakker
Copy link
Contributor

See lit/lit#515 for a minimal reproduction.

I dont see this happen for all shared imports. Possibly it has something to do with the export *

@Westbrook
Copy link

Also seeing this, any thoughts towards a fix or areas I might support investigating towards one?

@Westbrook
Copy link

Indeed @LarsDenBakker it does seem to be somehow related to the export * in that when you update this line in lit-html https://github.com/Polymer/lit-html/blob/master/src/lit-html.ts#L26 to be:

export { directive, isDirective } from './lib/directive.js';

The error is avoided.

@Westbrook
Copy link

I debugged around on this a bunch over the weekend, and while I'm not really closer to understanding how to fix this issue, I hope maybe someone with more experience in this area could maybe build off of my findings.

After doing some ridiculously naive debugging (if anyone has a good article or video on debugging this sort of code, I'd be ravenous to learn how to make this easier for myself, really I'm just dropping console.log all over the build and then running it over and over again), I found that in https://github.com/Polymer/tools/blob/master/packages/bundler/src/es6-module-utils.ts#L25 the directive import/export is process twice, once from lit-html.js and once from lib/directive.js, which makes sense due to the import * from ... usage. The two passes register the following data into the bundledExports and moduleExports:

moduleUrl /node_modules/lit-html/lit-html.js
name directive
exportName directive

moduleUrl /node_modules/lit-html/lib/directive.js
name directive
exportName directive$1

This would be all well and good, except with the storage of directive from lit-html.js which actually gets mutated later into a compound export of {directive, isDirective} as $directive there is no longer a direct export of directive for external fragments to rely on. By this application it would seem that later imports of directive should be compiled to $directive.directive which certainly isn't happening. An alternate approach would be to revisit the difference between name and exportName in remaining exports when the second mutation occurs (possibly in https://github.com/Polymer/tools/blob/master/packages/bundler/src/es6-rewriter.ts#L465), but I'm not 100% sure where, yet.

It's not 100% clear if this is supposed to be avoided by the fact that the build is also emitting the directive$1 from directive.js which would theoretically act as the transitive application of the code after the removal of the * context. However, that would require the import of directive when used in later fragments to be updated to directive$1, which also isn't happening.

In that it looks like I can work around this in my project by avoiding the use of directives for right now, I'm not sure how much more time I can put into correcting this issue in the near term. In our world of conference driven development, I hope the possibility of a 1.0 release of lit-html/lit-element (@justinfagnani) at CDS (in just 4 week, 😱😀🎉) might bump this up in priority. In support of that, I'd be happy to revisit my findings above with anyone who happened to be looking into this, as I'm sure the ramblings above make less sense than they could.

@usergenic
Copy link
Contributor

There is a bug in rollup specifically related to the use of export * from (apparently, only where referencing an external module, i.e. not part of the bundle) that I filed recently when tracking this down: rollup/rollup#2489

I'm looking at possible workarounds for this within bundler. One of them is essentially what you demonstrated, which is performing a translation of the export * from to an explicit named export {a, b, c} from as part of a before-I-hand-it-off-to-rollup step. I may have a new PR within a couple of days or just include it as part of #722.

@Westbrook
Copy link

@usergenic were you able to get the workaround you envisioned into #722? And, do you have an idea when it might be part of a release? Unless I'm doing something wrong, which is always possible, neither versions 1.8.1 nor 1.9.0-pre.2 of polymer-cli seem to be building this correctly. Let me know if there is anything I can do to support getting this into the world!

@LarsDenBakker
Copy link
Contributor

Can we get a release? This is a really nasty bug that we'd really like to have a fix for...

@keanulee
Copy link
Contributor Author

Fixed in polymer-cli 1.9.0

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

No branches or pull requests

4 participants