-
-
Notifications
You must be signed in to change notification settings - Fork 9
refactor(loader-webpack): refactored aysnc await code to use promises #34
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,9 +14,10 @@ export class TextTemplateLoader { | |
| * @param entry The TemplateRegistryEntry to load and populate with a template. | ||
| * @return A promise which resolves when the TemplateRegistryEntry is loaded with a template. | ||
| */ | ||
| async loadTemplate(loader: Loader, entry: TemplateRegistryEntry) { | ||
| const text = await loader.loadText(entry.address); | ||
| entry.template = DOM.createTemplateFromMarkup(text); | ||
| loadTemplate(loader: Loader, entry: TemplateRegistryEntry) { | ||
| return loader.loadText(entry.address).then((text) => { | ||
| entry.template = DOM.createTemplateFromMarkup(text); | ||
| }) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -52,7 +53,7 @@ export class WebpackLoader extends Loader { | |
| loaderPlugins = Object.create(null) as { [name: string]: LoaderPlugin & { hot?: (moduleId: string) => void } }; | ||
| modulesBeingLoaded = new Map<string, Promise<any>>(); | ||
| templateLoader: TextTemplateLoader; | ||
| hmrContext: { | ||
| hmrContext: { | ||
| handleModuleChange(moduleId: string, hot: Webpack.WebpackHotModule): Promise<void>, | ||
| handleViewChange(moduleId: string): Promise<void> | ||
| }; | ||
|
|
@@ -63,27 +64,31 @@ export class WebpackLoader extends Loader { | |
| this.useTemplateLoader(new TextTemplateLoader()); | ||
|
|
||
| this.addPlugin('template-registry-entry', { | ||
| fetch: async (moduleId: string) => { | ||
| fetch: (moduleId: string) => { | ||
| // HMR: | ||
| if (module.hot) { | ||
| if (!this.hmrContext) { | ||
| // Note: Please do NOT import aurelia-hot-module-reload statically at the top of file. | ||
| // We don't want to bundle it when not using --hot, in particular in production builds. | ||
| // Webpack will evaluate the `if (module.hot)` above at build time | ||
| // Webpack will evaluate the `if (module.hot)` above at build time | ||
| // and will include (or not) aurelia-hot-module-reload accordingly. | ||
| const { HmrContext } = require('aurelia-hot-module-reload'); | ||
| this.hmrContext = new HmrContext(this as any); | ||
| } | ||
| module.hot.accept(moduleId, async () => { | ||
| await this.hmrContext.handleViewChange(moduleId); | ||
| module.hot.accept(moduleId, () => { | ||
| return this.hmrContext.handleViewChange(moduleId).then((resource) => { | ||
| return resource; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why return |
||
| }); | ||
| }); | ||
| } | ||
|
|
||
| const entry = this.getOrCreateTemplateRegistryEntry(moduleId); | ||
| if (!entry.templateIsLoaded) { | ||
| await this.templateLoader.loadTemplate(this, entry); | ||
| return this.templateLoader.loadTemplate(this, entry).then(() => { | ||
| return entry; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| }); | ||
| } | ||
| return entry; | ||
| return Promise.resolve(entry); | ||
| } | ||
| } as LoaderPlugin); | ||
|
|
||
|
|
@@ -100,27 +105,29 @@ export class WebpackLoader extends Loader { | |
| }; | ||
| } | ||
|
|
||
| async _import(address: string, defaultHMR = true) { | ||
| _import(address: string, defaultHMR = true) { | ||
| const addressParts = address.split('!'); | ||
| const moduleId = addressParts.splice(addressParts.length - 1, 1)[0]; | ||
| const loaderPlugin = addressParts.length === 1 ? addressParts[0] : null; | ||
|
|
||
| if (loaderPlugin) { | ||
| const plugin = this.loaderPlugins[loaderPlugin]; | ||
| if (!plugin) { | ||
| throw new Error(`Plugin ${loaderPlugin} is not registered in the loader.`); | ||
| return Promise.reject(new Error(`Plugin ${loaderPlugin} is not registered in the loader.`)); | ||
| } | ||
| if (module.hot && plugin.hot) { | ||
| module.hot.accept(moduleId, () => plugin.hot!(moduleId)); | ||
| } | ||
| return await plugin.fetch(moduleId); | ||
| return plugin.fetch(moduleId).then((resource) => { | ||
| return resource; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question as above: isn't |
||
| }); | ||
| } | ||
|
|
||
| if (__webpack_require__.m[moduleId]) { | ||
| if (defaultHMR && module.hot && this.hmrContext) { | ||
| module.hot.accept(moduleId, () => this.hmrContext.handleModuleChange(moduleId, module.hot)); | ||
| } | ||
| return __webpack_require__(moduleId); | ||
| return Promise.resolve(__webpack_require__(moduleId)); | ||
| } | ||
|
|
||
| const asyncModuleId = `async!${moduleId}`; | ||
|
|
@@ -131,10 +138,10 @@ export class WebpackLoader extends Loader { | |
| module.hot.accept(asyncModuleId, () => {}); | ||
| } | ||
| const callback = __webpack_require__(asyncModuleId) as (callback: (moduleExports: any) => void) => void; | ||
| return await new Promise(callback); | ||
| return new Promise(callback); | ||
| } | ||
|
|
||
| throw new Error(`Unable to find module with ID: ${moduleId}`); | ||
| return Promise.reject(new Error(`Unable to find module with ID: ${moduleId}`)); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -188,21 +195,23 @@ export class WebpackLoader extends Loader { | |
| * @param moduleId The module ID to load. | ||
| * @return A Promise for the loaded module. | ||
| */ | ||
| async loadModule(moduleId: string, defaultHMR = true) { | ||
| loadModule(moduleId: string, defaultHMR = true) { | ||
| let existing = this.moduleRegistry[moduleId]; | ||
| if (existing) { | ||
| return existing; | ||
| return Promise.resolve(existing); | ||
| } | ||
| let beingLoaded = this.modulesBeingLoaded.get(moduleId); | ||
| if (beingLoaded) { | ||
| return beingLoaded; | ||
| } | ||
| beingLoaded = this._import(moduleId, defaultHMR); | ||
| this.modulesBeingLoaded.set(moduleId, beingLoaded); | ||
| const moduleExports = await beingLoaded; | ||
| this.moduleRegistry[moduleId] = ensureOriginOnExports(moduleExports, moduleId); | ||
| this.modulesBeingLoaded.delete(moduleId); | ||
| return moduleExports; | ||
| // const moduleExports = await beingLoaded; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't leave comments behind, git history is there for that. |
||
| return beingLoaded.then((moduleExports) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just saying: when you've got a single argument, braces are optional (I find it more legible without). |
||
| this.moduleRegistry[moduleId] = ensureOriginOnExports(moduleExports, moduleId); | ||
| this.modulesBeingLoaded.delete(moduleId); | ||
| return moduleExports; | ||
| }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -219,13 +228,14 @@ export class WebpackLoader extends Loader { | |
| * @param url The url of the text file to load. | ||
| * @return A Promise for text content. | ||
| */ | ||
| async loadText(url: string) { | ||
| const result = await this.loadModule(url, false); | ||
| if (result instanceof Array && result[0] instanceof Array && result.hasOwnProperty('toString')) { | ||
| // we're dealing with a file loaded using the css-loader: | ||
| return result.toString(); | ||
| } | ||
| return result; | ||
| loadText(url: string) { | ||
| return this.loadModule(url,false).then((result) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Space in |
||
| if (result instanceof Array && result[0] instanceof Array && result.hasOwnProperty('toString')) { | ||
| // we're dealing with a file loaded using the css-loader: | ||
| return result.toString(); | ||
| } | ||
| return result; | ||
| }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing |
||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing
;here