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

Upgrade to Babel 7 #10293

Merged
merged 1 commit into from
Nov 23, 2018
Merged

Upgrade to Babel 7 #10293

merged 1 commit into from
Nov 23, 2018

Conversation

wojtekmaj
Copy link
Contributor

@wojtekmaj wojtekmaj commented Nov 23, 2018

Updated using babel-upgrade with custom fixes on top, mostly for custom Babel configuration.

I don't know how to verify it better, gulp generic passes without any error, I ran the viewer and it seems to be fine.
Happy to hear your comments :)

@wojtekmaj wojtekmaj changed the title Babel 7 Upgrade to Babel 7 Nov 23, 2018
package.json Outdated
@@ -1,21 +1,24 @@
{
"name": "pdf.js",
"version": "2.0.0",
"main": "build/lib/pdf.js",
Copy link
Collaborator

@Snuffleupagus Snuffleupagus Nov 23, 2018

Choose a reason for hiding this comment

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

This must be reverted, since that's not at all desired/correct.

Please remember that the lib folder mostly exists for running the unit-tests on Travis, and the officially supported way of using the library is through the built pdf.js/pdf.worker.js files.

Also, please remember to squash the commits, since landing ones that fail linting/testing is not allowed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, sorry, fixed that.

Update configuration to work with Babel 7
Explicitly require globals - eslint-plugin-mozilla needs it, but doesn't require it on its own.
Fix Regexp to match Babel 7's inlined _interopRequireDefault
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.67.70.0:8877/86dc347ce8aad2a/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/86dc347ce8aad2a/output.txt

Total script time: 3.08 mins

Published

@timvandermeij timvandermeij merged commit b9b8cef into mozilla:master Nov 23, 2018
@timvandermeij
Copy link
Contributor

Nice work!

@wojtekmaj wojtekmaj deleted the babel-7 branch February 17, 2019 11:48
timvandermeij added a commit to timvandermeij/pdf.js that referenced this pull request Jul 29, 2024
This dependency got introduced in PR mozilla#10293, almost six years ago now,
because `eslint-plugin-mozilla` didn't work without it but also didn't
require it as a dependency itself.

However, nowadays `eslint-plugin-mozilla` works just fine without it,
and other dependencies that need it correctly require it themselves.
This can be seen using `npm ls globals`:

```
$ npm ls globals
pdf.js
├─┬ @babel/core@7.24.9
│ └─┬ @babel/traverse@7.25.0
│   └── globals@11.12.0
├─┬ @babel/preset-env@7.25.0
│ └─┬ @babel/plugin-transform-classes@7.25.0
│   └── globals@11.12.0
├─┬ eslint-plugin-unicorn@55.0.0
│ └── globals@15.8.0 deduped
├─┬ eslint@8.57.0
│ ├─┬ @eslint/eslintrc@2.1.4
│ │ └── globals@13.24.0
│ └── globals@13.24.0
└── globals@15.8.0
```

Further proof that `eslint-plugin-mozilla` (no longer) uses `globals` is
from a source code search in
https://searchfox.org/mozilla-central/search?q=globals&path=&case=false&regexp=false.
The only results for `eslint-plugin-mozilla` refer to a file named
`globals.js`, but the `globals` NPM package is not actually imported
anywhere.

Given this we should be able to safely get rid of this explicit
dependency on our end now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants