Option "browserTarget" is deprecated. Option "buildTarget" should be used instead.

[jordimarimon]

Angular CLI v17 deprected in the @angular-devkit/build-angular:extract-i18n builder the browserTarget in favor of the new buildTarget option.

If one executes the ng extract-i18n command using the ng-extract-i18n-merge:ng-extract-i18n-merge builder with Angular CLI version 17, they will get the following warning:

Running ng-extract-i18n-merge for project <project_name>
running "extract-i18n" ...
Option "browserTarget" is deprecated: Use 'buildTarget' instead.

If one changes in the angular.json in the extract-i18n architect the option browserTarget to buildTarget, they will get the following error:

Error: Schema validation failed with the following errors:
Data path "" must have required property 'browserTarget'.

I have never written a custom builder for Angular but I suppose that the warning comes because of this line:

https://github.com/daniel-sc/ng-extract-i18n-merge/blob/master/src/builder.ts#L110

And the error comes because of the schema.json:

https://github.com/daniel-sc/ng-extract-i18n-merge/blob/master/src/schema.json#L113-L117

One can still use the ng-extract-i18n-merge:ng-extract-i18n-merge builder with Angular CLI v17 as the option has only been deprecated, not removed.

It's only to give a heads up, for when Angular removes it completely.

Maybe in the future one cool thing would be to have an schematic for this builder that could be executed with ng update and it would internally call the update schematic of @angular-devkit/build-angular:extract-i18n to make sure that all users have the builder options updated. But this would required a lot of work. I would be open to help though.

[daniel-sc]

@jordimarimon thanks for this detailed input (and sorry for letting this slide for some time..)!

There are some relatively independent points to address:

1. Use "buildTarget" when invoking the original angular i18n builder - and fall back to "browserTarget" for older versions.
2. Add new config option "buildTarget" for this plugin - and keep "browserTarget" for compatibility. Probably we'd need to make them both optional..
3. Create a new major version, were fallbacks are removed and an automated migration changes the config. This will only support Angular 17+. (Here buildTarget should become required again.)

There are imho two ways to proceed:
a) Implement 1+2 and delay 3 for some time, to keep compatibility with older angular versions for some time.
b) Directly implement 3 (skipping parallel/fallback versions) - this is less effort, but reduces compatiblity.

I'm not totally sure if a) or b) would be the smart move - any input?

[daniel-sc]

Reading https://angular.io/guide/releases#deprecation-practices I'd rather go for a) - at least if it does not induce too much overhead.. (inputs still welcome!)

[gongAll]

commented on your commit with some ideas, @daniel-sc -- thanks for your work!

[daniel-sc]

This is a mess - anyone an expert with "jest commonjs + esm dynamic imports"? See https://stackoverflow.com/questions/77962982/testing-commonjs-with-dynamic-imports-of-esm-with-ts-jest
Any help would be appreciated.. :)

[jordimarimon]

Hi Daniel!

Sorry for not answering before. When I saw the answer, it was already too late and a PR had already started.

I have made a fork of the project and the following commit to fix it:

https://github.com/jordimarimon/ng-extract-i18n-merge/commit/14795a2c3b00110c69b5774d2f55d5b24c17be97

I have been able to successfully execute both the build and the test scripts.

This a fix I made with just a few minutes, probably you will want to organize things better. Also feel free to remove the any type I have added in src/builder.spec.ts to a better one. I just was too lazy to define a correct type here. I can define the type for you If you don't find a good one.

These are the changes I have made to make it work:

• Use ts-jest/presets/default-esm preset for ESM compatibility -> more info here

• Add --experimental-vm-modules when executing jest -> more info here

• Because now all *.spec.ts files are ESM and not CommonJS, we don't have access to global variables like jest or __dirname. I have made the changes accordingly . For the __dirname I have used the following recommendations that you can find here based also in the versions you're testing in the github workflows. I don't know if you would like to remove Node 14 from the github workflow as it is not mantained anymore based on the Node releases. Maybe I'm wrong here.

• Finally I have added another tsconfig.json for the building process... The reason why we have now two tsconfig files is because for building we want CommonJS but for testing we want ESM. I don't like having multiple tsconfig files. In reality we are not forced to add another tsconfig file If we provide the tsconfig options for testing directly throught the jest.config.ts file. I didn't do that because if we do it, probably we will have to remove the "preset" and provide all the options that the preset sets (the options are documented here and also can be found here and here):

import {JestConfigWithTsJest} from 'ts-jest';

// Sync object
const config: JestConfigWithTsJest = {
    preset: 'ts-jest/presets/default-esm', // -> This will need to be removed
    testEnvironment: 'node',
    verbose: false,
    maxWorkers: 100, // -> maybe we don't need this large value anymore?
    maxConcurrency: 1,
    testTimeout: 30_000,
    testMatch: undefined,
    testRegex: '.*\.spec\.ts$',
    collectCoverageFrom: ['src/**/*.ts'], // exclude coverage from schematic as it only collects from js (instead of ts)..
    coveragePathIgnorePatterns: ['src/rmSafe.ts'],
    
    // HERE PROVIDE THE TSCONFIG OPTIONS SO MODULE IS NOT COMMONJS
    transform: {
        '^.+\\.tsx?$': [
      'ts-jest',
      {
        useESM: true,
        tsconfig: {
             // Provide the same that right now is in `tsconfig.json`
            // https://kulshekhar.github.io/ts-jest/docs/getting-started/options#options
        },
      },
    ],
    },
};
export default config;

[jordimarimon]

Also If you would like to disable Node warnings (due to the use of experimental features) when executing tests, we can change (probably won't work in Windows as I think it has another way of setting environment variables):

NODE_OPTIONS="$NODE_OPTIONS --experimental-vm-modules" jest

with:

NODE_OPTIONS="$NODE_OPTIONS --experimental-vm-modules" NODE_NO_WARNINGS=1 jest

[daniel-sc]

@jordimarimon thanks very much for picking this up!
There is only one thing, I feel a little unsure about: How much risk do we have when diverging the module system in test and productive code? (And all this only for importing the angular version..) The alternative could be to make a major release and drop support for v16 (and before..)?

[jordimarimon]

@daniel-sc

Like you said in your comment, this is a mess due to the fact that we have to build for CommonJS but test with ESM...

This shouldn't be the case. I would only support Node 18 and Node 20. I wouldn't support older versions of Node or Angular versions that are no longer mantained.

This would make easier to move to ESM entirely and stop using CommonJS as I think it's time to move on.

The changes would be the following:

• Add "type": "module" in the the package.json. That way all JS files are treated as ESM by default by Node.
• Change module and moduleResolution in the tsconfig.json file to Node16 or NodeNext (I prefer NodeNext). This will make useless the necessity of havin two tsconfig files.
• The change in the tsconfig.json would require rewritting all imports to include the file extension (.js). This is how modern Node works. I think it's a good change and makes it easier to work with other tools in the ecosystem. Here you can see how it looks like.
• Also I would add the following properties in the tsconfig file:
  • isolatedModules: true -> better compatibility with other bundlers that don't use the TypeScript compiler (like esbuild or SWC)
  • useDefineForClassFields: true -> to make sure the TypeScript code is valid JS code. This would mean stop using decorators as parameters as of right now.
  • verbatimModuleSyntax: true -> I do like this because it gives more information to bundlers that don't use the TypeScrupt compiler. But it's only useful if you use another bundler for transpiling TS and not the TypeScript compiler.

With all theses changes we would have a modern project with TypeScript and ESM and we could even stop using Jest and all the hacks that make it work with TypeScript + ESM and just use Vitest (which it's 100% compatible with Jest). You wouldn't need presets or transforms. I have already tested Vitest with this project and it just works out of the box and you have support for coverage as well.

The only problem that right now makes it impossible to migrate to TypeScript and ESM is this line of code in the Angular CLI repo:

https://github.com/angular/angular-cli/blob/main/packages/angular_devkit/schematics/tools/export-ref.ts#L22-L25

Because AngularCLI is still using require() we can't publish this project with ESM because require() can't be used to load an ESM module. It would be nice if the Angular CLI stopped using require() and used import() instead.

[jordimarimon]

@daniel-sc

The above comment is more a suggestion for the future, not for right now as it's not possible to stop supporting CommonJS sadly...

[jordimarimon]

There is only one thing, I feel a little unsure about: How much risk do we have when diverging the module system in test and productive code? (And all this only for importing the angular version..) The alternative could be to make a major release and drop support for v16 (and before..)?

Yes, you're right. Probably we would need a major version just to "notify" people.

I wil open an issue in the Angular CLI asking what it would take to stop using code that makes it not possible for third party builders to distribute ESM modules.

I want to know if it's possible for the Angular CLI to support ESM third party builders or maybe I'm missing something.

For example if the Angular CLI loads builders dynamically using require(), it could check first if the builder is ESM and in case of being ESM use a dynamic import, otherwise use require().

I thinks this is the same that it's being done here although I'm not entirely sure.

I have to do a little bit of research and also see the source code of the Angular CLI to understand better how everything works right now. Everything I'm saying right now comes from my ignorance and my lack of understanding of how the Angular CLI works internally.

[daniel-sc]

@jordimarimon there is already an open issue: angular/angular-cli/issues/22786

[daniel-sc]

I might have found a cleaner way: import the schema.json of the angular-devkit i18n-builder and check for the attribute existence there. This way, we don't need to import the angular version and problems with CommonJS/ESM interop go away.. (of course, when/if angular allows for ESM schematics/builders we should migrate eventually) - see #104

[daniel-sc]

It seems the other approach works well - can some of you please check out the beta version 2.11.0-1 and report if anything is broken?

[daniel-sc]

This is resolved with v2.11.0