-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
Angular 13 compatibility #247
base: master
Are you sure you want to change the base?
Conversation
@bartholomej I've tested it with Angular 13 NodeJS 16 as well but still doesn't work. Gives me the same error as before your changes. |
Can confirm that this changes do not work node:internal/modules/cjs/loader:936
throw err;
^
Error: Cannot find module '../dist/cli/cli'
Require stack:
- <path>/node_modules/@biesbjerg/ngx-translate-extract/bin/cli.js
at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
Angular 13 NodeJS 16 |
@bartholomej i've opened a pull request in your repo to merge my changes into your branch. The new changes fix the issues. |
@venraij node:internal/modules/cjs/loader:936
throw err;
^
Error: Cannot find module '../dist/cli/cli.js'
Require stack:
-<path>\node_modules\@biesbjerg\ngx-translate-extract\bin\cli.js
at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
at Function.Module._load (node:internal/modules/cjs/loader:778:27)
|
Ok made the build and copied the dist folder but still not working node:internal/modules/cjs/loader:979
throw new ERR_REQUIRE_ESM(filename, true);
^
Error [ERR_REQUIRE_ESM]: require() of ES Module <path>\node_modules\@angular\compile
r\fesm2015\compiler.mjs not supported.
Instead change the require of <path>\node_modules\@angular\compiler\fesm2015\compile
r.mjs to a dynamic import() which is available in all CommonJS modules.
at Object.<anonymous> (<path>\node_modules\@biesbjerg\ngx-translate-extract\dist
\parsers\pipe.parser.js:3:20)
at Object.<anonymous> <path>\node_modules\@biesbjerg\ngx-translate-extract\dist
\cli\cli.js:25:23)
at Object.<anonymous> (<path>\node_modules\@biesbjerg\ngx-translate-extract\bin\
cli.js:3:1) {
code: 'ERR_REQUIRE_ESM'
}
|
@tiberiuzuld give this a whirl https://www.npmjs.com/package/@misternick/ngx-translate-extract it is the packaged version. It uses the code in this branche of my repo https://github.com/venraij/ngx-translate-extract/tree/ng13 |
Still the same issue node:internal/modules/cjs/loader:979
throw new ERR_REQUIRE_ESM(filename, true);
^
Error [ERR_REQUIRE_ESM]: require() of ES Module <path>\node_modules\@angular\compile
r\fesm2015\compiler.mjs not supported.
Instead change the require of <path>\node_modules\@angular\compiler\fesm2015\compile
r.mjs to a dynamic import() which is available in all CommonJS modules.
at Object.<anonymous> (<path>\node_modules\@misternick\ngx-translate-extract\dis
t\parsers\pipe.parser.js:3:20)
at Object.<anonymous> (<path>\node_modules\@misternick\ngx-translate-extract\dis
t\cli\cli.js:25:23)
at Object.<anonymous> (<path>\node_modules\@misternick\ngx-translate-extract\bin
\cli.js:3:1) {
code: 'ERR_REQUIRE_ESM'
}
Maybe the issue in my case is the fact that I use |
So after some more trial and error arrived at this changes: My initial issue starts from the change in angular packages which now default to ESM so all the libraries depending on angular need to make the switch angular/angular#43833 My changes still don't work completely at least for my project.
Issues that I encountered along the way that required changes in the code changes linked above:
// before
.version(require(__dirname + '/../../package.json').version)
// after
.version(process.env.npm_package_version) The changes are complex to upgrade to ng13 and the last error I hit when running the changes in my project doesn't offer any clues where to take it from there. |
Update: The issues: Where from: protected expressionIsOrHasBindingPipe(exp: any): exp is BindingPipe {
if (exp && exp.name && exp.name === TRANSLATE_PIPE_NAME) {
return true;
}
if (exp && exp.exp && exp.exp instanceof BindingPipe) {
return this.expressionIsOrHasBindingPipe(exp?.exp);
}
return false;
} Seems Made a new PR with my changes which work with ng13: #248 Also made a branch on my fork with the dist folder for who wants to use it directly in
|
@tiberiuzuld I've made some further changes which fixed the issue even via npm for me. Tried it in 2 different projects of ng13. https://github.com/venraij/ngx-translate-extract/tree/angular-13 Unfortunately it breaks compatibility with lower angular versions because of the removal of MethodCall |
I have the same problem as @enchorb |
@adnanomerovic Temporary solution using patch-package and this fix:
|
@biesbjerg could we get some feedback from you? |
It works on:
|
does not work for me |
@enchorb @adnanomerovic @orestisioakeimidis You're right. This should be fixed in version npm i @bartholomej/ngx-translate-extract --save-dev Let me know how it works. |
@pongells Can you be a little more specific, please? What doesn't work? Any error messages? Are you using latest version of ngx-translate-extract |
It's works! :D |
8.0.2 was not out at the time :) works with latest version, thank you |
If there are no problems any more, maybe this PR can me merged? |
@zeljkoantich Yes, that's what we all need 😺 |
… to solve angular 13 compatibility `npm uninstall @biesbjerg/ngx-translate-extract` `npm i @bartholomej/ngx-translate-extract` see: biesbjerg/ngx-translate-extract#246 biesbjerg/ngx-translate-extract#247 angular/angular#43833 (comment)
… to solve angular 13 compatibility `npm uninstall @biesbjerg/ngx-translate-extract` `npm i @bartholomej/ngx-translate-extract` see: biesbjerg/ngx-translate-extract#246 biesbjerg/ngx-translate-extract#247 angular/angular#43833 (comment)
Good job @bartholomej! Come on @biesbjerg can we please have some feedback from you? |
thanks @bartholomej! |
@bartholomej Maybe you should you release your fork officially, then we would switch to that fork? Another alternative would be to have you here as a contributor with all rights. The lib is used, and there are no official commits in over a year. |
@bartholomej your extractor is working on angular 13 but I am encountering a problem with output path using curly braces. My out file path has this path in the end: "/{en,jp}.json". Running the extractor will create a file "{en,jp}.json" at my output folder. Checking the file, the extraction was a success. So the only problem is that its saving the output in an incorrect filename. |
Are you on Windows? On Windows I must specify each output destination individually: |
Works good! Thank you! |
Is this a new limitation? Yes I am on Windows and your suggestion solves my issue but I didn't encounter this issue while I was using version 7.0.4 on Windows. |
For me
|
I currently use both |
@FrEaKmAn Yes, I am using both Although |
Is there any reason for this to be held up any more? Everyone in the comments seems to be agreeing that things work? Or is this repo considered deprecated and we should move to another offering? |
@timeimp This repo is unmaintained. After consultation with the interested parties, we took on maintainership of a fork. You can read the discussion here: #246 (comment) |
What is in this PR?
Why?
Because it no longer works with Angular 13. Issue: #246
Notes
More testing would be appreciated :)