Skip to content

fix: properly handle dynamic System.import on the client - #5

Open
pkozlowski-opensource wants to merge 1 commit into
caridy:masterfrom
pkozlowski-opensource:issue4
Open

fix: properly handle dynamic System.import on the client#5
pkozlowski-opensource wants to merge 1 commit into
caridy:masterfrom
pkozlowski-opensource:issue4

Conversation

@pkozlowski-opensource

Copy link
Copy Markdown

Fixes #4

@pkozlowski-opensource

Copy link
Copy Markdown
Author

@caridy I was trying to add proper tests for this, but from my exploration of the tests in this project, my understanding is that I could only add manual tests for the client-side part, right?

If so I would need a bit of input from you as current manual tests seem to be "broken" - we are referencing a non-existing c.js in a <script> tag - is it a bug / left-over or does it serve some purpose?

Comment thread client.js

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I see the problem here, but removing the around promise is not the solution since that promise protects issues with normalizeName and get.

The problem here is really that resolve is never called when load is invoked. The solution should be to propagate resolve and reject a then() when calling load().

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.

Top-level promise for System.import not resolved for dynamically loaded files

2 participants