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

BREAKING CHANGE(core, cli, schema) Update deps, drop support for Node versions older than 10 #218

Merged
merged 3 commits into from
May 14, 2020

Conversation

xavdid
Copy link
Contributor

@xavdid xavdid commented May 14, 2020

This PR is a result of me running yarn upgrade-interactive --latest repeatedly until everything was where I wanted it.

I largely left legacy-scriptring-runner alone

I changed all our root devDeps to lock to the major version. This will let us update more often and since they're the dev deps, it's less important that we ship an exact version. This means we have to trust folks not to break semver, but we mostly use bigger projects, so i'm not too worried. This will let us run yarn upgrade periodically and it'll update all our devDeps for us.

Oh, and I took your advice and locked yarn onto a specific version. It does that by downloading the entire thing in a single file and then we check it in. That's why the diff is so huge.

@xavdid xavdid requested a review from eliangcs as a code owner May 14, 2020 01:07
@@ -3,3 +3,4 @@ package.json
package-lock.json
packages/cli/snippets/next.js
packages/cli/src/generators/templates/
.yarn
Copy link
Contributor Author

Choose a reason for hiding this comment

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

prettier was hanging for a long time on my first pre-commit because it was trying to prettier all of yarn 🙈

},
"workspaces": [
"packages/*",
"example-apps/*"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't think we need to include these anymore.

],
"*.{js,json}": [
"prettier --write",
"git add"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

newer version of lint-staged handles git add for us, so the docs said to remove it.

const mockUpload = bodyMatcher => {
nock('https://s3-fake.zapier.com')
.post('/', bodyMatcher)
.reply(204, '');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removing the '' was the only actual change here. That's the default in nock now, so I didn't figure we needed it.

@xavdid
Copy link
Contributor Author

xavdid commented May 14, 2020

We can't bump jsonschema because there was a breaking change in 1.2.3: tdegrunt/jsonschema#261

I remember that we're doing something sort of unusual w/ our jsonschema (that was what was originally blocking the work to turn the schema into a typescript def), so we'll leave it where it is for now.

@xavdid
Copy link
Contributor Author

xavdid commented May 14, 2020

ok, tests look ✅, I feel good about it. feel free to merge this if you're 👍

Copy link
Member

@eliangcs eliangcs left a comment

Choose a reason for hiding this comment

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

Yep, looks good!

@eliangcs eliangcs merged commit a6ac842 into master May 14, 2020
@eliangcs eliangcs deleted the update-deps-v10 branch May 14, 2020 05:38
@awwright
Copy link

awwright commented Oct 2, 2020

Hi @xavdid and company, I released 1.2.8 of jsonschema which should fix the issue you're encountering.

I'm curious, would you be able to point me to the code where you're validating a property somewhere up the prototype chain?

@xavdid
Copy link
Contributor Author

xavdid commented Oct 30, 2020

Hey @awwright, appreciate the follow-up! I just tried it and it looks like 1.2.8 doesn't fix the change introduced in 1.2.3. When running tests in our schema package of this repo, I'm seeing SchemaError: no such schema </DynamicFieldsSchema> for a number of tests. That seems like the only schema broken (all now-failing tests reference it) so my guess is that we were relying undocumented behavior that was changed.

I wouldn't worry about it too much on your end- it's not critical that we keep the latest version of jsonschema.

If you do want to play around, you can reproduce like so:

git clone git@github.com:zapier/zapier-platform.git
cd zapier-platform
yarn
cd packages/schema
yarn test
# edit packages/schema/package.json to use jsonschema 1.2.3
yarn test

Tests pass using 1.2.2 and fail on 1.2.3+. But again, I wouldn't worry too much about it!

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.

None yet

3 participants