Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 39 additions & 29 deletions src/aurelia-loader-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing ; here

}
}

Expand Down Expand Up @@ -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>
};
Expand All @@ -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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why return resource?
.then(x => x) is a no-op, isn't it?

});
});
}

const entry = this.getOrCreateTemplateRegistryEntry(moduleId);
if (!entry.templateIsLoaded) {
await this.templateLoader.loadTemplate(this, entry);
return this.templateLoader.loadTemplate(this, entry).then(() => {
return entry;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

() => entry seems shorter?

});
}
return entry;
return Promise.resolve(entry);
}
} as LoaderPlugin);

Expand All @@ -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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same question as above: isn't .then(res => res) a no-op?

});
}

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}`;
Expand All @@ -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}`));
}

/**
Expand Down Expand Up @@ -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;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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;
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing ;

}

/**
Expand All @@ -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) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Space in url,false was lost.

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;
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing ;

}

/**
Expand Down