Skip to content
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

can you do a release? #1242

Closed
jcesarmobile opened this issue Feb 3, 2021 · 18 comments · Fixed by #1243
Closed

can you do a release? #1242

jcesarmobile opened this issue Feb 3, 2021 · 18 comments · Fixed by #1243
Assignees

Comments

@jcesarmobile
Copy link

With npm 7 being default, npm install command fails on apps that use typescript 4 because latest version of tsickle has a peerDependency of typescript 3.
It was changed to typescript 4 on #1213, but it isn't released.

While it's possible to use --force or --legacy-peer-deps params on npm install to workaround the problem, would be good if the package could be released.

@evmar
Copy link
Contributor

evmar commented Feb 3, 2021

We can make a release, but often when we hear from users in your state it's because they accidentally depend upon tsickle when they didn't mean to. Can you confirm that you actually are using tsickle? How?

@brad4d brad4d self-assigned this Feb 3, 2021
@jcesarmobile
Copy link
Author

I'm not, it's a peerDependency of ng-packagr, which is a peerDependency of @angular-devkit/build-angular which is the one I use. And both of them have typescript 4 as a peerDependency.

@evmar
Copy link
Contributor

evmar commented Feb 3, 2021

@alxhub Do you know why ng-packagr has this dep?

@alxhub
Copy link
Member

alxhub commented Feb 3, 2021

@kyliau might know. It's probably legacy, from the days when tsickle was a hard dependency of @angular/compiler-cli. It hasn't been for a while, so likely the right answer is for ng-packagr to remove the peer dep.

@kyliau
Copy link
Contributor

kyliau commented Feb 4, 2021

I don't see any reason ng-packagr still needs tsickle. @alan-agius4 could you please enlighten us?

@clydin
Copy link
Member

clydin commented Feb 4, 2021

Both tsickle and ng-packagr are optional peer dependencies of ng-packagr and @angular-devkit/build-angular, respectively. This appears to be an npm 7 defect in that it shouldn't be attempting to install either of them.

@jcesarmobile
Copy link
Author

It a npm 7 feature, it installs peer dependencies if you don’t pass --force or --legacy-peer-deps params to the install command.

@clydin
Copy link
Member

clydin commented Feb 4, 2021

Peer dependencies should be installed when using npm 7+ but optional peer dependencies should not. Otherwise they are no longer optional peer dependencies.

@clydin
Copy link
Member

clydin commented Feb 4, 2021

As to the ng-packagr usage of tsickle, it is listed as an optional peer dependency for developers wishing to use the annotateForClosureCompiler option. The optional peer dependency helps to ensure that a compatible version of tsickle is used with the Angular compiler in such cases.

@clydin
Copy link
Member

clydin commented Feb 4, 2021

After some additional experimentation via the --force option, it appears that the optional peer dependencies are not installed. However, they are still being resolved/analyzed and npm is refusing to install at all even if they are not present in the project.

@jcesarmobile
Copy link
Author

ng-packagr removed the tsickle dependency, so this is no longer a problem for angular users, but will leave the issue open.

Feel free to close.

@brad4d
Copy link
Contributor

brad4d commented Feb 11, 2021

I attempted to release following these instructions:

https://github.com/angular/tsickle#releasing

The final command failed.

$ bazel run :npm_package.publish -- --registry https://wombat-dressing-room.appspot.com
2021/02/11 12:55:03 Downloading https://releases.bazel.build/4.0.0/release/bazel-4.0.0-linux-x86_64...
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
ERROR: REDACTED_HOME/.cache/bazel/_bazel_bradfordcsmith/7bac4c144fec2608461228dfd1a39ffb/external/build_bazel_rules_nodejs/internal/node/node.bzl:87:45: invalid escape sequence: \/. You can enable unknown escape sequences by passing the flag --incompatible_restrict_string_escapes=false
ERROR: REDACTED_HOME/.cache/bazel/_bazel_bradfordcsmith/7bac4c144fec2608461228dfd1a39ffb/external/build_bazel_rules_nodejs/internal/node/node.bzl:87:64: invalid escape sequence: \.. You can enable unknown escape sequences by passing the flag --incompatible_restrict_string_escapes=false
ERROR: error loading package '': in REDACTED_HOME/.cache/bazel/_bazel_bradfordcsmith/7bac4c144fec2608461228dfd1a39ffb/external/build_bazel_rules_nodejs/index.bzl: Extension 'internal/node/node.bzl' has errors
INFO: Elapsed time: 5.091s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded)
FAILED: Build did NOT complete successfully (0 packages loaded)

@brad4d brad4d reopened this Feb 11, 2021
@brad4d
Copy link
Contributor

brad4d commented Feb 11, 2021

I retried with this command:

bazel run :npm_package.publish --incompatible_restrict_string_escapes=false -- --registry https://wombat-dressing-room.appspot.com

That seemed to work up to the point that it couldn't find a publish key.
Which makes sense, because I don't have one.

@brad4d
Copy link
Contributor

brad4d commented Feb 11, 2021

I'll probably need help from @evmar to finish this.

leifjones pushed a commit to leifjones/ng2-idle that referenced this issue Feb 19, 2021
Based on finding that tsickle is a blocker to ng update'ing to 11, and the discussion [here](angular/tsickle#1242)
@annervisser
Copy link

@evmar Any update on this?
I am using tsickle to compile a typescript project with closure. (using tscc)
The 0.39.0 of tsickle release is currently the blocking factor in updating to typescript 4.x, a new release would allow tscc to update as well.

I am able to build & run my application with [email protected] by building tsickle manually at 43f59d1 and applying a small patch to tscc

@brad4d brad4d assigned mprobst and unassigned brad4d Jun 1, 2021
@rohitpratapitm
Copy link

Still facing the same issue while creating a new application using angular cli and npm v7.15.1

@angular/cli 11.2.x // has a peerDependency of typescript <4.1
@tsickle 0.39.1 // has a peerDependency of typescript ~3.8.2

Same issue with tsickle v 0.40.0 as well. There the peer Dependency is typescript ~4.2

Since npm v7 installs peer dependencies automatically, it is getting into version conflict issue. See error below:

> npm "install"

(node:15608) ExperimentalWarning: The fs.promises API is experimental
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/typescript
npm ERR!   dev typescript@"~4.1.5" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer typescript@"~3.8.2" from [email protected]
npm ERR! node_modules/tsickle
npm ERR!   dev tsickle@"0.39.0" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.

@mprobst
Copy link
Contributor

mprobst commented Jun 7, 2021

@rohitpratapitm can you file an issue in https://github.com/angular/angular? I believe it's a bug in Angular if new applications still depend on tsickle.

leifjones pushed a commit to leifjones/ng2-idle that referenced this issue Jul 4, 2021
Based on finding that tsickle is a blocker to ng update'ing to 11, and the discussion [here](angular/tsickle#1242)
leifjones pushed a commit to leifjones/ng2-idle that referenced this issue Jul 7, 2021
Based on finding that tsickle is a blocker to ng update'ing to 11, and the discussion [here](angular/tsickle#1242)
leifjones pushed a commit to leifjones/ng2-idle that referenced this issue Jul 7, 2021
Based on finding that tsickle is a blocker to ng update'ing to 11, and the discussion [here](angular/tsickle#1242)
@brad4d
Copy link
Contributor

brad4d commented Jul 27, 2021

At this point I think it's clear that we aren't going to do any more releases.

@brad4d brad4d closed this as completed Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants