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

Update dependencies for npm-app and protobuf import #6253

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

achamayou
Copy link
Member

@achamayou achamayou commented Jun 12, 2024

Attempt at resolution for:

62: src/endpoints/all.ts → dist/src...
62: [!] (plugin typescript) Error: @rollup/plugin-typescript TS2307: Cannot find module 'protobufjs/dist/protobuf.js' or its corresponding type declarations.
62: src/endpoints/proto.ts (2:22)
62: 
62: 2 import protobuf from "protobufjs/dist/protobuf.js";
62:                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
62: 
62: Error: @rollup/plugin-typescript TS2307: Cannot find module 'protobufjs/dist/protobuf.js' or its corresponding type declarations.
62:     at error (/__w/1/s/tests/npm-app/node_modules/rollup/dist/shared/rollup.js:198:30)
62:     at throwPluginError (/__w/1/s/tests/npm-app/node_modules/rollup/dist/shared/rollup.js:21718:12)
62:     at Object.error (/__w/1/s/tests/npm-app/node_modules/rollup/dist/shared/rollup.js:22672:20)
62:     at emitDiagnostic (/__w/1/s/tests/npm-app/node_modules/@rollup/plugin-typescript/dist/index.js:540:17)
62:     at reportDiagnostics (/__w/1/s/tests/npm-app/node_modules/@rollup/plugin-typescript/dist/index.js:548:9)
62:     at Array.forEach (<anonymous>)
62:     at emitFilesAndReportErrors (/__w/1/s/tests/npm-app/node_modules/typescript/lib/typescript.js:126138:21)
62:     at result.afterProgramCreate (/__w/1/s/tests/npm-app/node_modules/typescript/lib/typescript.js:126329:13)
62:     at Object.afterProgramCreate (/__w/1/s/tests/npm-app/node_modules/@rollup/plugin-typescript/dist/index.js:703:29)
62:     at synchronizeProgram (/__w/1/s/tests/npm-app/node_modules/typescript/lib/typescript.js:126639:22)

https://dev.azure.com/MSRC-CCF/CCF/_build/results?buildId=85452&view=logs&j=f3457e4b-102d-5a4f-f801-dc119e4853a8&t=ae98f324-492e-5969-593b-258ede9c5463

Although this produces worrying warnings, it appears to work at least locally:

55: (!) Circular dependencies
55: node_modules/protobufjs/src/util/minimal.js -> node_modules/protobufjs/src/util/longbits.js -> node_modules/protobufjs/src/util/minimal.js
55: node_modules/protobufjs/src/object.js -> node_modules/protobufjs/src/util.js -> node_modules/protobufjs/src/type.js -> node_modules/protobufjs/src/namespace.js -> node_modules/protobufjs/src/object.js
55: node_modules/protobufjs/src/object.js -> node_modules/protobufjs/src/util.js -> node_modules/protobufjs/src/type.js -> node_modules/protobufjs/src/namespace.js -> node_modules/protobufjs/src/field.js -> node_modules/protobufjs/src/object.js
55: ...and 22 more
55: (!) Use of eval is strongly discouraged
55: https://rollupjs.org/troubleshooting/#avoiding-eval

Note: the browser files are still produced, but have moved, and so this is still possible:

// Importing the browser bundle works around https://github.com/protobufjs/protobuf.js/issues/1402.
import protobuf from "protobufjs/light.js";

but it does not improve the warning situation:

55: (!) Export "*" of module "node_modules/protobufjs/src/mapfield.js" was reexported through module "
55: /home/amchamay/CCF/tests/npm-app/node_modules/protobufjs/src/mapfield.js?commonjs-proxy" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.
55: Either change the import in "node_modules/protobufjs/src/index-light.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.
55: /home/amchamay/CCF/tests/npm-app/node_modules/protobufjs/src/index-light.js
55: (!) Export "*" of module "node_modules/protobufjs/src/mapfield.js" was reexported through module "
55: /home/amchamay/CCF/tests/npm-app/node_modules/protobufjs/src/mapfield.js?commonjs-proxy" while both modules are dependencies of each other and will end up in different chunks by current Rollup settings. This scenario is not well supported at the moment as it will produce a circular dependency between chunks and will likely lead to broken execution order.
55: Either change the import in "node_modules/protobufjs/src/type.js" to point directly to the exporting module or do not use "output.preserveModules" to ensure these modules end up in the same chunk.
55: /home/amchamay/CCF/tests/npm-app/node_modules/protobufjs/src/type.js
55: (!) Circular dependencies
55: node_modules/protobufjs/src/util/minimal.js -> node_modules/protobufjs/src/util/longbits.js -> node_modules/protobufjs/src/util/minimal.js
55: node_modules/protobufjs/src/object.js -> node_modules/protobufjs/src/util.js -> node_modules/protobufjs/src/type.js -> node_modules/protobufjs/src/namespace.js -> node_modules/protobufjs/src/object.js
55: node_modules/protobufjs/src/object.js -> node_modules/protobufjs/src/util.js -> node_modules/protobufjs/src/type.js -> node_modules/protobufjs/src/namespace.js -> node_modules/protobufjs/src/field.js -> node_modules/protobufjs/src/object.js
55: ...and 22 more
55: (!) Use of eval is strongly discouraged
55: https://rollupjs.org/troubleshooting/#avoiding-eval

@achamayou achamayou requested a review from a team June 12, 2024 08:27
@eddyashton eddyashton mentioned this pull request Jun 12, 2024
@achamayou achamayou merged commit 38a8afd into microsoft:main Jun 12, 2024
20 of 21 checks passed
@achamayou achamayou added auto-backport Automatically backport this PR to LTS branch 4.x-todo PRs which should be backported to 4.x labels Jun 13, 2024
@ghost
Copy link

ghost commented Jun 13, 2024

💔 All backports failed

Status Branch Result
release/4.x Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

backport --pr 6253

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

achamayou added a commit to achamayou/CCF that referenced this pull request Jun 13, 2024
(cherry picked from commit 38a8afd)

# Conflicts:
#	tests/npm-app/package.json
@achamayou achamayou added the backported This PR was successfully backported to LTS branch label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4.x-todo PRs which should be backported to 4.x auto-backport Automatically backport this PR to LTS branch backported This PR was successfully backported to LTS branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants