Skip to content
This repository was archived by the owner on Feb 6, 2023. It is now read-only.

Conversation

Tonours
Copy link

@Tonours Tonours commented Dec 12, 2017

Summary

Add a yarn.lock file at the root of the project.
Replace NPM with Yarn in travis.yml file.

Test Plan

Ensure that travis run without errors.
All current npm scripts has been manually tested (prepublish, pretest, build, dev, postbuild, lint, format, flow, test, test-ci), there are many lint warning on test files (see attached screenshots).

lint-error

  • get yarn set up for yourself
  • cd draft-js && rm -rf node_modules && yarn install
  • verify that builds, lint, tests, etc. are passing
  • commit the resulting yarn.lock file that gets added to the root of the repo
  • update .travis.yml to use yarn instead of npm for all commands. Note that the yarn commands may vary a bit from the npm CLI commands.

Add yarn.lock at the root of the project
in order to fix depencies versions.
Accordingly to the transition of NPM to Yarn
travis.yml file has been updated in order to use Yarn.
@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@Tonours Tonours mentioned this pull request Dec 12, 2017
6 tasks
@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Copy link
Contributor

@juliankrispel juliankrispel left a comment

Choose a reason for hiding this comment

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

lgtm 👍

Copy link
Contributor

@zpao zpao left a comment

Choose a reason for hiding this comment

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

I was going to come along and do this but you beat me to it :) Thanks!

Should we end up with a yarn.lock in website/ as well? I think there's a reference to npm in that package.json as well. Could you also make sure the website continues to build? It should but I've seen stranger things happen…

.travis.yml Outdated
# Update runs install, but will also pull in the latest updates so the cache
# doesn't get stale
- npm update
- yarn upgrade
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not keep this step - it's not relevant with yarn usage and locked dependencies, and in fact might cause problems. It was originally there to catch the case where npm deps would get updates and our dependency ranges would result in breakages that we didn't see. Locks prevent that.

@mitermayer
Copy link
Contributor

Hi @Tonours,

Thanks for submitting this PR, if you can amend the comments above from Paul I then should be able to land this.

Update travis.ci in order to remove install process
in order to remove yarn upgrade.
Update website directory in order to add a yarn.lock file.
Package.json of website has been updated in order to add
draft-js local package.
Postinstall script from website package.json has been removed
in order to prevent yarn loop over installation.
@Tonours
Copy link
Author

Tonours commented Dec 15, 2017

Hi @zpao and @mitermayer !

Good catch for website package.json and yarn upgrade in travis @zpao 👍
So, the website now have a yarn.lock file :-)

@Tonours Tonours changed the title Issue 1568 yarn to npm Issue 1568 npm to yarn Dec 15, 2017
@mitermayer
Copy link
Contributor

mitermayer commented Dec 15, 2017

@zpao , Now that we will move from npm to yarn, can we do something about this line on our package.json ?

"devEngines": {
    "npm": "2.x || 3.x"
}

Can we either remove that npm engine or bump it to latest npm ?

If so it would make sense to include it on this PR

@mitermayer
Copy link
Contributor

Also @Tonours, do you mind rebasing your pull request with master to fix your segfault ?

Summary:
This PR is part of a series of PR's that will be exploring **tree data block support** in Draft.

**Adding support for splitting transactions**

This PR adds support for tree block nodes splitting transactions

***

**Note:**
This is unstable and not part of the public API and should not be used by
production systems.
Closes facebookarchive#1548

Differential Revision: D6554958

fbshipit-source-id: 7d8e13a4f3494c81dfead78ba57a54ec82d6c06f
…anges from content state

Summary:
This PR is part of a series of PR's that will be exploring **tree data block support** in Draft.

**Adding support for removing content state**

This PR adds support for removing range transactions for ContentBlockNodes

***

**Note:**
This is unstable and not part of the public API and should not be used by
production systems.
Closes facebookarchive#1547

Differential Revision: D6555163

fbshipit-source-id: 7a2197541da062dbf8cd5c6974c4ecd1140e8e01
Summary:
- Testing to see if we still get segfaults
Closes facebookarchive#1575

Differential Revision: D6581397

fbshipit-source-id: 8c808550284ec292c9e327247e6c2f0c5ee58f2e
@mitermayer
Copy link
Contributor

mitermayer commented Dec 16, 2017

Looks like the build is failing in travis due to that npm version on our package.json

yarn run v1.3.2
$ node node_modules/fbjs-scripts/node/check-dev-engines.js package.json
assert.js:42
  throw new errors.AssertionError({
  ^
AssertionError [ERR_ASSERTION]: Current npm version is not supported for development, expected "5.5.1" to satisfy "2.x || 3.x".

Update npm requirements in order to satisfy 5.x version.
@Tonours
Copy link
Author

Tonours commented Dec 18, 2017

Hi @mitermayer !

Thanks for your feedback 👍
I've updated npm devEngines requirements for npm.
Let me know if everything is good for you.

@mitermayer
Copy link
Contributor

I want to merge this since we need a yarn.lock to fix our builds, just want to make sure that @zpao is fine with the changes to devEnvines to be pointing to "npm": "5.x" if so I will go ahead and merge this

cc @flarnie @zpao

mitermayer and others added 7 commits January 5, 2018 10:23
Add yarn.lock at the root of the project
in order to fix depencies versions.
Accordingly to the transition of NPM to Yarn
travis.yml file has been updated in order to use Yarn.
Update travis.ci in order to remove install process
in order to remove yarn upgrade.
Update website directory in order to add a yarn.lock file.
Package.json of website has been updated in order to add
draft-js local package.
Postinstall script from website package.json has been removed
in order to prevent yarn loop over installation.
Update npm requirements in order to satisfy 5.x version.
Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@mitermayer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Feb 16, 2018
Summary:
**Summary**

Fixes #1650.

Travis is using Node 8 since 247a632, and Prettier is already configured in a way that's not Node-6 compatible:

https://github.com/facebook/draft-js/blob/a6317e60b06519d3c00a2c0621701f3da0837a88/prettier.config.js#L3

Node 8 is the LTS release line since late october 2017 so I think it's fair to remove support for Node 6.

NB: I see `devEngines` also states support for npm 2 and 3. Node 8 ships with npm 5 by default, so I imagine those 2 versions could be removed as well. Edit: Ah actually I see draft-js is at least partially using yarn since #1568 / #1570.

**Test plan**

No tests. This is only removing support, it doesn't affect supported versions.
Closes #1652

Differential Revision: D7013709

fbshipit-source-id: bdb7e105352be2e8f205c36f516b1e6c4cedb591
alicayan008 pushed a commit to alicayan008/draft-js that referenced this pull request Jul 4, 2023
Summary:
**Summary**

Add a yarn.lock file at the root of the project.
Replace NPM with Yarn in travis.yml file.

**Test Plan**

Ensure that travis run without errors.
All current npm scripts has been manually tested (prepublish, pretest, build, dev, postbuild, lint, format, flow, test, test-ci), there are many lint warning on test files (see attached screenshots).

<img width="886" alt="lint-error" src="https://user-images.githubusercontent.com/1331358/33870374-7e061cca-dedb-11e7-8d16-baad0cb9f434.png">

- [x] get yarn set up for yourself
- [x] `cd draft-js && rm -rf node_modules && yarn install`
- [x] verify that builds, lint, tests, etc. are passing
- [x] commit the resulting `yarn.lock` file that gets added to the root of the repo
- [x] update `.travis.yml` to use `yarn` instead of `npm` for all commands. Note that the `yarn` commands may vary a bit from the npm CLI commands.
Closes facebookarchive/draft-js#1570

Differential Revision: D6667926

fbshipit-source-id: 20a1134352f7cfdb4257b6f9092f4143158d761f
aforismesen added a commit to aforismesen/draft-js that referenced this pull request Jul 12, 2024
Summary:
**Summary**

Add a yarn.lock file at the root of the project.
Replace NPM with Yarn in travis.yml file.

**Test Plan**

Ensure that travis run without errors.
All current npm scripts has been manually tested (prepublish, pretest, build, dev, postbuild, lint, format, flow, test, test-ci), there are many lint warning on test files (see attached screenshots).

<img width="886" alt="lint-error" src="https://user-images.githubusercontent.com/1331358/33870374-7e061cca-dedb-11e7-8d16-baad0cb9f434.png">

- [x] get yarn set up for yourself
- [x] `cd draft-js && rm -rf node_modules && yarn install`
- [x] verify that builds, lint, tests, etc. are passing
- [x] commit the resulting `yarn.lock` file that gets added to the root of the repo
- [x] update `.travis.yml` to use `yarn` instead of `npm` for all commands. Note that the `yarn` commands may vary a bit from the npm CLI commands.
Closes facebookarchive/draft-js#1570

Differential Revision: D6667926

fbshipit-source-id: 20a1134352f7cfdb4257b6f9092f4143158d761f
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants