VadimDez / ng2-pdf-viewer

πŸ“„ PDF Viewer Component for Angular
https://vadimdez.github.io/ng2-pdf-viewer/
MIT License
1.3k stars 416 forks source link

Angular 10 issue -> optimization bailouts #624

Open Seharad opened 4 years ago

Seharad commented 4 years ago

WARNING in /../node_modules/ng2-pdf-viewer/__ivy_ngcc__/fesm2015/ng2-pdf-viewer.js depends on pdfjs-dist/build/pdf. CommonJS or AMD dependencies can cause optimization bailouts. For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

JohnnyTMD commented 4 years ago

same here

PedroS5499 commented 4 years ago

same here, but put in angular.json like they said in official guide

[https://angular.io/guide/build#configuring-commonjs-dependencies]

"options": {
                        "allowedCommonJsDependencies": [",
                            "moment-business",
                            "ng2-pdf-viewer",
                            "pdfjs-dist/build/pdf",
                            "pdfjs-dist/web/pdf_viewer"
                        ],
seopei commented 4 years ago

this is just for ignoring the warnings. But the goal should be to get rid of the warnings in general.

Seharad commented 4 years ago

Ignoring errors is not the way to go.

bekstn commented 4 years ago

same here, any news?

saithis commented 4 years ago

pdfjs is not written with es modules, but they made a es module wrapper available that could be used in the meantime: https://www.npmjs.com/package/@bundled-es-modules/pdfjs-dist

for more info: https://github.com/mozilla/pdf.js/issues/10317

VadimDez commented 4 years ago

@saithis thanks for pointing the right issue @bundled-es-modules/pdfjs-dist seems to be outdated

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mrmokwa commented 3 years ago

Can we re-open this one?

PedroS5499 commented 3 years ago

@mrmokwa yes indeed

PedroS5499 commented 3 years ago

any news?

mememelm commented 3 years ago

I hate this Warning :'(

raichshlv commented 3 years ago

Any updates?

edusoccer1121 commented 3 years ago

Any updates on this?

oussemazouaghi commented 3 years ago

This warning is really annoying any pending fixes for this yet?

imvikaskohli commented 3 years ago

I m able to fix the issue by adding commonJsdependency property in the angular.json

angular.json

Inside Build command there is an option "build": { "builder": "@angular-devkit/build-angular:browser", "options": { "allowedCommonJsDependencies": ["ng2-pdf-viewer"], }

Seharad commented 3 years ago

I m able to fix the issue by adding commonJsdependency property in the angular.json

angular.json

Inside Build command there is an option "build": { "builder": "@angular-devkit/build-angular:browser", "options": { "allowedCommonJsDependencies": ["ng2-pdf-viewer"], }

this is not a fix, this is an ignore.

stephanrauh commented 3 years ago

@VadimDez When I tried to build you library from source, I ran into the same problem. Even worse: the require() statements work in the ng2-pdf-viewer demo project, but they failed to work after copying the dist folder to my test project.

Can we get rid of the require() statements? Angular's stopped supporting require.js. It'd be nice to use an import statement instead. Or we could load pdf.js and viewer.js directly. That'd allow for loading these big files lazily.

nikhil-kapoor-quo commented 3 years ago

any updates?

VadimDez commented 3 years ago

all require()s have been removed

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

turbolego commented 3 years ago

@VadimDez If i understand this correctly, unless an ECMAScript module bundle version of ng2-pdf-viewer is released, we have to add ng2-pdf-viewer to allowedCommonJsDependencies to ignore this warning? πŸ€”

Do you think an ECMAScript module bundle version of ng2-pdf-viewer will be available in the future? πŸ”­

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

MikeMatusz commented 3 years ago

Bump to remove stale.

SKhalidQ commented 3 years ago

Has anyone actually found a solution to this? Even if its temporary workaround?

nischi commented 3 years ago

https://github.com/mozilla/pdf.js/issues/10317

Still open :( Hope they will provide it.

hal2814 commented 3 years ago

Not to sound like a broken record, but has there been any progress on this issue?

korvised commented 2 years ago

first create an ngcc.config.js file on the root => https://githubmemory.com/repo/VadimDez/ng2-pdf-viewer/issues/754

and then allowedCommonJsDependencies

"allowedCommonJsDependencies": [ "ng2-pdf-viewer", "pdfjs-dist" ]

it work for my project

gultyayev commented 2 years ago

Any progress? It seems like this type of module causes my initial bundle to have extra 500 Kb, while I'm using it only in lazy loaded modules so I'm expecting it not to be present in an initial bundle

psarno commented 2 years ago

Also hoping for any updates on this.

\node_modules\ng2-pdf-viewer__ivy_ngcc__\fesm2015\ng2-pdf-viewer.js depends on 'pdfjs-dist/es5/build/pdf'. CommonJS or AMD dependencies can cause optimization bailouts. For more info see: https://angular.io/guide/build#configuring-commonjs-dependencies

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

nischi commented 2 years ago

nono not close πŸ˜‚

jacwallace commented 2 years ago

Is using https://www.npmjs.com/package/@bundled-es-modules/pdfjs-dist still a viable solution for this issue?

psarno commented 2 years ago

Our project is clean outside of this particular issue.

Is this being worked on?

NashIlli commented 2 years ago

+1 any news? Thanks!

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

NashIlli commented 2 years ago

nono not close πŸ˜‚

PedroS5499 commented 2 years ago

nono not close πŸ˜‚

πŸ˜‚

egonknapen commented 2 years ago

I get the message fesm2020 depends on 'pdfjs-dist/legacy/build/pdf

I could be wrong, but in the readme of pdfjs-dist it says, that this version is older browsers or environments without the support for modern features such as async/wait support.

So the only way to remove that message, would be to start using the modern pdfjs version in pdfjs-dist/build/pdf and not the legacy in ng2-pdf-viewer. Of course with the usual upgrade problems.

Is this a viable option, or planning in the near future?

SKhalidQ commented 2 years ago

Ngl I don't even remember what this was about 🀣

egonknapen commented 2 years ago

It's the same on angular 13, it just complains about the optimization bailout when you do a ng-serve. It's still something that needs to be fixed in the foreseeable future and not ignored.

remy33 commented 2 years ago

In short it has to be addressed since it prevent effective trees-shake and eventually add over 150KB to the GZIPPED package.

armaancsnapper commented 2 years ago

Any progress?

mrmokwa commented 2 years ago

This doesn't depend only from VadimDez... This issue probably won't be ever done

https://github.com/mozilla/pdf.js/issues/10317#issuecomment-1112675400

...we don't want to add even more builds and we also want to support both browsers and Node.js environments with the same builds.

gultyayev commented 2 years ago

Node is slowly but surely transitioning from commonjs to modules

stephanrauh commented 2 years ago

@VadimDez I could donate the lazy loading algorithm of my own library, ngx-extended-pdf-viewer. The disadvantage of my approach is there's no tree shaking. The huge pdf.js files are loaded as-is. On the plus side, you get lazy loading. In most cases, this outweighs the disadvantage of having bigger files.

Interested?

Best regards, Stephan

remy33 commented 2 years ago

Sounds good, as long as it doesn't add weight to the main package the zise doesn't matter. Zoom meeting for example is nearly 5MB but allow lazy loading.

Since PDF has weight, user won't expect expect loading to be immediately.

On Tue, May 10, 2022, 21:59 Stephan Rauh @.***> wrote:

@VadimDez https://github.com/VadimDez I could donate the lazy loading algorithm of my own library, ngx-extended-pdf-viewer. The disadvantage of my approach is there's no tree shaking. The huge pdf.js files are loaded as-is. On the plus side, you get lazy loading. In most cases, this outweighs the disadvantage of having bigger files.

Interested?

Best regards, Stephan

β€” Reply to this email directly, view it on GitHub https://github.com/VadimDez/ng2-pdf-viewer/issues/624#issuecomment-1122756150, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD2TRA6LHHJODL2ZZPVGDBTVJKW2LANCNFSM4OIVAKTQ . You are receiving this because you commented.Message ID: @.***>

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

SKhalidQ commented 2 years ago

Hahahaha nah g you ain't gonna do that. This is gonna stay open until it doesnΒ΄t get fixed

duwaljyotioc commented 2 years ago

Its not stale.

I am having issue as well.