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.

Sign up for GitHub

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

Jump to bottom

fix: fix build and output cleanup #702

Merged
merged 2 commits into from
Feb 2, 2023
Merged

fix: fix build and output cleanup #702

merged 2 commits into from
Feb 2, 2023

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Feb 2, 2023

fix #701

@skjnldsv skjnldsv self-assigned this Feb 2, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 2, 2023
Merged
artonge
artonge approved these changes Feb 2, 2023
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we storing the pdfjs files in this repo and not using a node module for security reason ?

package.json Outdated
"dev": "npm run pdfjs:get && NODE_ENV=development webpack --progress --config webpack.js",
"watch": "npm run pdfjs:get && NODE_ENV=development webpack --progress --watch --config webpack.js",
"prebuild": "rm -rf js && npm run pdfjs:get",
"build": "webpack --node-env production --progress",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird indendation

@szaimen
Copy link
Collaborator

szaimen commented Feb 2, 2023 •
edited
Loading

@skjnldsv seems like the dependabot is the reason why the files were missing. See e.g.: #704

szaimen
szaimen previously requested changes Feb 2, 2023
View reviewed changes
Copy link
Collaborator

@szaimen szaimen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a rebase and new compile

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 2, 2023

Are we storing the pdfjs files in this repo and not using a node module for security reason ?

Yes, we're not bundling this library. We're using it as is.

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 2, 2023 •
edited
Loading

@skjnldsv seems like the dependabot is the reason why the files were missing. See e.g.: #704

No, the dependabot is doing properly.
The issue is he webpack vue that is now cleaning the output before building.
So removing the pdfjs dist files we manually downloaded before.

files_pdfviewer/webpack.config.js

Lines 7 to 8 in 6288127

// keep pdfjs vendor in the js folder
webpackConfig.output.clean = false

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 2, 2023

/rebase

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 2, 2023

/compile amend /

@skjnldsv
fix: fix build and output cleanup
dd057ee 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
Update template from source
fc11fbb 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv mentioned this pull request Feb 2, 2023
Merged
@skjnldsv skjnldsv merged commit 58f5261 into master Feb 2, 2023
@skjnldsv skjnldsv deleted the fix/build branch February 2, 2023 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

@szaimen szaimen szaimen left review comments

Assignees

@skjnldsv skjnldsv

Labels
4. to release bug regression
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: pdf.js not loading in 26
3 participants
@skjnldsv @szaimen @artonge