Skip to content

refactor: dynamic import for assets - #2233

Closed
saschanaz wants to merge 20 commits into
developfrom
karma-module2
Closed

refactor: dynamic import for assets#2233
saschanaz wants to merge 20 commits into
developfrom
karma-module2

Conversation

@saschanaz

@saschanazsaschanaz commented Apr 7, 2019

Copy link
Copy Markdown
Collaborator

Currently blocked by marked dependency in core/utils:

  • ES modules cannot reference a package via its package name
  • marked does not have a ES module distribution

The first problem can be fixed by import maps but currently only Chrome beta 74 implements it. The second one is being worked on but it seems not to be active now.

Resolved by #2240

@marcoscaceres

Copy link
Copy Markdown
Contributor

Amazing :) If you want to send this in pieces, it would be awesome.

@saschanaz
saschanaz marked this pull request as ready for review April 8, 2019 08:55
@saschanaz

Copy link
Copy Markdown
CollaboratorAuthor

Will soon send some smaller pieces from this.

@saschanaz

Copy link
Copy Markdown
CollaboratorAuthor

@marcoscaceres This contains asset loading part and karma part... The former is okay, but the latter requires Chrome because it's the only browser that supports dynamic import now. Should we wait until Firefox stable enables dynamic import?

@marcoscaceres

Copy link
Copy Markdown
Contributor

Should we wait until Firefox stable enables dynamic import?

No, I don't think we should wait... If we want to test against Firefox, we could figure out how to enable them via setting javascript.options.dynamicImport.

@marcoscaceres

Copy link
Copy Markdown
Contributor

Bingo: https://github.com/karma-runner/karma-firefox-launcher/blob/master/README.md#custom-preferences

@saschanazsaschanaz changed the title refactor: use ES module in karma testsrefactor: dynamic import for assetsApr 9, 2019
@saschanaz

Copy link
Copy Markdown
CollaboratorAuthor

Closing for now, because this should really be a part of #2187 or a fix for #1640, and both are not ready yet.

Sign up for freeto 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.

2 participants

@saschanaz@marcoscaceres