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

🐠️ babel@7 📦️ upgrades #31

Merged
merged 9 commits into from
Jan 9, 2019
Merged

Conversation

JeromeFitz
Copy link
Contributor

@JeromeFitz JeromeFitz commented Dec 28, 2018

🐠️ babel@7

  • ⬆️ Upgrade from babel@6

📦️ package(-lock).json

  • ⬆️ upgrades
  • 🔥️ removal of unused (or handled by next)

@JeromeFitz
Copy link
Contributor Author

Tried to do this as 1:1 as possible to keep the PR size down. (Some other enhancements I have may be more preference rather than function and would want to get more input on.)

Tests all pass and build is working. This may be a decent first step for prep with #4.

 PASS  src/meta/__tests__/buildTags.spec.jsx
  ✓ renders correctly (35ms)
  ✓ returns full array for default seo object (55ms)
  ✓ correctly sets noindex, nofollow (9ms)
  ✓ displays title with titleTemplate integrated (13ms)
  ✓ Article SEO renders correctly (8ms)
  ✓ Check article og type meta (10ms)
  ✓ Book SEO renders correctly (12ms)
  ✓ Check book og type meta (11ms)
  ✓ Profile SEO renders correctly (6ms)
  ✓ Check profile og type meta (6ms)

Test Suites: 1 passed, 1 total
Tests:       10 passed, 10 total
Snapshots:   4 passed, 4 total
Time:        2.094s

@JeromeFitz
Copy link
Contributor Author

😅 Whew.

@JeromeFitz JeromeFitz mentioned this pull request Dec 28, 2018
@garmeeh
Copy link
Owner

garmeeh commented Dec 28, 2018

Hey @JeromeFitz thanks very much for the PR 💪

I haven't had a chance just yet to have a look, but hope to get it over the weekend.

@JeromeFitz
Copy link
Contributor Author

Very cool.

As a heads up, in testing some stuff locally I was inadvertently using the test:e2e and not test:e2e:dev. 😬

Sorry about that. I look to have blown up the spot in the free tier for December. 😞

Copy link
Owner

@garmeeh garmeeh left a comment

Choose a reason for hiding this comment

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

Hey @JeromeFitz, just testing this out and everything is looking good. Just have some questions.

taskfile.js Outdated Show resolved Hide resolved
taskfile.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
🐠️ babel@7
- ⬆️ Upgrade from babel@6

✨️ taskr
- Instead of babel/cli

📦️ package.json
- ⬆️ upgrades

TODO: Verify Major Version Upgrades of:
- eslint-plugin-jest@22.1.2
- react-testing-library@5.4.2
- regenerator-runtime@0.13.1
⬆️
- eslint-plugin-jest@22.1.2
- react-testing-library@5.4.2
- regenerator-runtime@0.13.1

📈️
Test Suites: 1 passed, 1 total
Tests:       10 passed, 10 total
Snapshots:   4 passed, 4 total
This should get travis working.
Cypress is not keen on babel.config.js (babel@7) just yet.
We probably don't want these in the repo. :octocat:
- Prefer @babel/cli over taskr
- Move __tests__ as they are not being ignored
@JeromeFitz
Copy link
Contributor Author

Thanks for looking into this @garmeeh !

Removed taskr and went back to using babel's cli like we had before.

One caveat, I couldn't get the .spec.js files not to compile. 😬 So I moved them to the root for now. If we want to keep the tests files together, I can spend a little more time on it.

`next@7.0.2-canary.35` was the implementation
- Dedupe only items with unique key: [5800](vercel/next.js#5800)

In upgrading all the packges in this PR, some
 versions were not working (not sure why).

I've tested with the latest `canary` from `next`
 and everything is passing.
🐠️ next/babel
Since we rely on `next` they handle presets
 through `next/babel` and we can modify
 if we want. (skeleton left in place)

🔥️ unused packages

These are all handled by `next`:
- @babel/runtime (`regenerator-runtime`)
- @babel/preset-env
- @babel/preset-react
- babel-core
- babel-loader

And not used by `next-seo` I believe.

`flatten` should have been removed via `taskr` commit
@JeromeFitz JeromeFitz changed the title 🐠️ babel@7 ✨️ taskr 📦️ upgrades 🐠️ babel@7 📦️ upgrades Jan 6, 2019
Copy link
Owner

@garmeeh garmeeh left a comment

Choose a reason for hiding this comment

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

Looking good 💪, just two small comments. Once they are addressed I am happy to merge 😄

If the suggested change works for excluding the tests from compiling, let's move the tests back.

package.json Outdated Show resolved Hide resolved
babel.config.js Show resolved Hide resolved
📦️ #.#.# = Specific Versions
- Was having a terrible time with `npm install` and `^` in package.json.
- Is it cool to do this for now?
- - I didn't want to just move to `yarn` because I was running into npm troubles

🐠️ babel
Added:
- babel-core@7.0.0-bridge.0 => needed until jest moves to babel@7
- babel-minify
Presets:
- Removed the empty objects

🃏️ jest
- Putting in the blank values was breaking the tests

🔥️ @babel/plugin-proposal-object-rest-spread

🚚️ test
- Moved __tests__ back
- Have an extra `rimraf` removing them from the `build`.
@JeromeFitz
Copy link
Contributor Author

JeromeFitz commented Jan 8, 2019

Can't seem to dismiss, let me know if you'd like anything else. 👍

Added babel-minify and re-ran the tests and the e2e:test:dev. Everything is looking good.

Should reduce the size a bit: packagephobia?next-seo

Copy link
Owner

@garmeeh garmeeh left a comment

Choose a reason for hiding this comment

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

💪 Nice one!

@garmeeh garmeeh merged commit d931dad into garmeeh:master Jan 9, 2019
@JeromeFitz
Copy link
Contributor Author

Word up. Thanks for going over everything! 💯️

@JeromeFitz JeromeFitz deleted the babel@7 branch January 9, 2019 15:08
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 this pull request may close these issues.

2 participants