Skip to content

fix(@ngtools/webpack): emit require in replace resources when using CommonJS as module#18719

Merged
alan-agius4 merged 1 commit intoangular:masterfrom
alan-agius4:unreferenced-import
Sep 8, 2020
Merged

fix(@ngtools/webpack): emit require in replace resources when using CommonJS as module#18719
alan-agius4 merged 1 commit intoangular:masterfrom
alan-agius4:unreferenced-import

Conversation

@alan-agius4
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 commented Sep 7, 2020

When using CommonJs as module format TypeScript will generate unreferenced require when using ts.createImportDeclaration.

const external_component_html_1 = require("!raw-loader!./external.component.html");
const core_1 = require("@angular/core");
let ExampleComponent = class ExampleComponent {
};
ExampleComponent = __decorate([
    core_1.Component({
        selector: 'example-compoent',
        template: __NG_CLI_RESOURCE__0,
    })
], ExampleComponent);

More context: microsoft/TypeScript#18369 (comment)

Closes #18718

@alan-agius4 alan-agius4 changed the title fix(@ngtools/webpack): emit require in replace resources when using… fix(@ngtools/webpack): emit require in replace resources when using CommonJS as module Sep 7, 2020
@alan-agius4 alan-agius4 added the target: patch This PR is targeted for the next patch release label Sep 7, 2020
@alan-agius4 alan-agius4 requested a review from clydin September 7, 2020 18:56
… CommonJS as module

When using CommonJs as module format TypeScript will generate unreferenced `require` when using `ts.createImportDeclaration`.

```js
const external_component_html_1 = require("!raw-loader!./external.component.html");
const core_1 = require("@angular/core");
let ExampleComponent = class ExampleComponent {
};
ExampleComponent = __decorate([
    core_1.Component({
        selector: 'example-compoent',
        template: __NG_CLI_RESOURCE__0,
    })
], ExampleComponent);
```

More context: microsoft/TypeScript#18369 (comment)

Closes #18718
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Sep 8, 2020
@alan-agius4 alan-agius4 merged commit 179b6ca into angular:master Sep 8, 2020
@alan-agius4 alan-agius4 deleted the unreferenced-import branch September 8, 2020 18:55
@ahnpnl
Copy link
Copy Markdown

ahnpnl commented Sep 30, 2020

hi @alan-agius4 , I have an issue which is introduced by this PR. Maybe worth to ask you here before opening an issue.

The issue is: jest-preset-angular wants to reuse the codes from ngtools/webpack. Currently I am reusing replace_resources transformer. The codes which are produced after this PR is

Object.defineProperty(exports, "__esModule", { value: true });
exports.AppComponent = void 0;
const tslib_1 = require("tslib");
const core_1 = require("@angular/core");
let AppComponent = class AppComponent {
    constructor() {
        this.title = 'test-app-v10';
    }
};
AppComponent = tslib_1.__decorate([
    core_1.Component({
        selector: 'app-root',
        template: require("./app.component.html").default,
        styles: []
    })
], AppComponent);
exports.AppComponent = AppComponent;

However, the line require('./app.component.html').default made the test failed with exception

Error: No template specified for component AppComponent

    at syntaxError (/Users/ahn/jest-preset-angular/e2e/packages/compiler/src/util.ts:108:17)
    at DirectiveNormalizer.normalizeTemplate (/Users/ahn/jest-preset-angular/e2e/packages/compiler/src/directive_normalizer.ts:85:13)
    at CompileMetadataResolver.loadDirectiveMetadata (/Users/ahn/jest-preset-angular/e2e/packages/compiler/src/metadata_resolver.ts:262:54)
    at /Users/ahn/jest-preset-angular/e2e/packages/compiler/src/jit/compiler.ts:137:36
    at Array.forEach (<anonymous>)
    at /Users/ahn/jest-preset-angular/e2e/packages/compiler/src/jit/compiler.ts:135:65
    at Array.forEach (<anonymous>)
    at JitCompiler._loadModules (/Users/ahn/jest-preset-angular/e2e/packages/compiler/src/jit/compiler.ts:132:71)
    at JitCompiler._compileModuleAndAllComponents (/Users/ahn/jest-preset-angular/e2e/packages/compiler/src/jit/compiler.ts:117:32)
    at JitCompiler.compileModuleAndAllComponentsAsync (/Users/ahn/jest-preset-angular/e2e/packages/compiler/src/jit/compiler.ts:69:33)
    at CompilerImpl.compileModuleAndAllComponentsAsync (/Users/ahn/jest-preset-angular/e2e/packages/platform-browser-dynamic/src/compiler_factory.ts:69:27)
    at TestingCompilerImpl.compileModuleAndAllComponentsAsync (/Users/ahn/jest-preset-angular/e2e/packages/platform-browser-dynamic/testing/src/compiler_factory.ts:59:27)
    at TestBedViewEngine.compileComponents (/Users/ahn/jest-preset-angular/e2e/packages/core/testing/src/test_bed.ts:375:27)
    at Function.TestBedViewEngine.compileComponents (/Users/ahn/jest-preset-angular/e2e/packages/core/testing/src/test_bed.ts:159:25)

If switching to require('./app.component.html') will make the test pass.

Is it a bug or is it intended behavior ? jest-preset-angular doesn't use webpack while Angular CLI does, so I can understand that the test https://github.com/angular/angular-cli/blob/master/packages/ngtools/webpack/src/transformers/replace_resources_spec.ts#L72 is a valid test.

My question is: is it possible that replace_resources transformer can decide when to use require('./app.component.html) or require('./app.component.html).default ?

@alan-agius4
Copy link
Copy Markdown
Collaborator Author

Hi @ahnpnl, using the mentioned transformer is a private API and is not meant to be used outside and without @ngtools/webpack.

@ahnpnl
Copy link
Copy Markdown

ahnpnl commented Sep 30, 2020

Thanks for the quick answer! I will just copy the source and modify in the way I want :)

@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Oct 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

__NG_CLI_RESOURCE__0 is not defined

4 participants