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

feat(npm-scripts): add jest mock for createRange and createContextualFragment #234

Merged
merged 5 commits into from
Nov 12, 2020

Conversation

bryceosterhaus
Copy link
Member

This PR has a handful of failing unit tests due to createRange not existing in js-dom. Rather than add a setupfile for each module to include this, I figured its best to have as a global mock.

liferay-frontend/liferay-portal#468

We generally consider lint suppressions to be a "smell", because they
can cover up real problems. So, use an alternative approach instead,
telling ESLint that this file runs in a "browser" context (where
`document` is defined).
Copy link
Contributor

@wincent wincent left a comment

Choose a reason for hiding this comment

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

lg2m. I'm going to apply some small tweaks on top and merge as soon as I see it green.

@wincent
Copy link
Contributor

wincent commented Nov 12, 2020

So I didn't test this but I assume you did @bryceosterhaus 😂 — there are any number of different ways createRange could be mocked, depending on the context (example here), so let's just assume that this is going to be good enough to meet our use cases until we do the update, and if it isn't, then we'll tweak it. It's green now, so I'm going to pull the trigger.

If you want to try cutting a release, the CONTRIBUTING.md should be up-to-date. Things have probably changed a bit since the last time you did it, but not too much. It's basically going to boil down to:

cd projects/npm-tools/packages/npm-scripts

# Preview the changelog changes, to decide on release type (major, minor etc),
# and stage them:
yarn run liferay-changelog-generator --interactive

# Update the version number using the same release type as decided in previous step:
yarn version --minor

@wincent wincent merged commit 32b5848 into liferay:master Nov 12, 2020
@bryceosterhaus
Copy link
Member Author

@wincent are there any docs for updating npm-scripts in portal?

@wincent
Copy link
Contributor

wincent commented Nov 16, 2020

@wincent are there any docs for updating npm-scripts in portal?

No because it is pretty straight forward; from modules/:

  1. yarn add -W --dev @liferay/npm-scripts@x.y.z
  2. Make sure lockfile has no unwanted duplication.
  3. Test/sanity check according to the changes in the release.

I can add this to the contributing doc though. The only tricky part is that sometimes Yarn requires a bit of coaxing to deduplicate the lockfile, as noted here:

Yarn is a bit fickle about these things, so there are four ways you can do this and not all of them always work.

  • npx yarn-deduplicate yarn.lock (didn't work here)
  • yarn install (worked for this case)
  • yarn install --force (not needed)
  • rm yarn.lock && yarn install (last resort, very aggressive)

I usually start at the first and go down that list until I get the result I want. It used to be that step 1 was always enough; but lately I've found I have to do 2 quite often as well. I might heav had to do 3 once, and I've never done 4.

@wincent
Copy link
Contributor

wincent commented Nov 16, 2020

Docs added here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants