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

Move esm to devDependencies #8513

Closed
wants to merge 1 commit into from
Closed

Move esm to devDependencies #8513

wants to merge 1 commit into from

Conversation

DatGreekChick
Copy link
Contributor

@DatGreekChick DatGreekChick commented Jul 18, 2019

Launch Checklist

  • briefly describe the changes in this PR
  • post benchmark scores
  • manually test the debug page

I simply ran npm i -D esm to move the package to devDependencies. This is the output I get from running tests:

// unit tests: my branch
total ......................................... 16491/16491

  16491 passing (3m)

  ok

// integration tests - my branch
440 passed (40.8%)
1 passed but were ignored (0.1%)
6 ignored (0.6%)
631 failed (58.5%)
1 errored (0.1%)

// integration tests - master
440 passed (40.8%)
1 passed but were ignored (0.1%)
6 ignored (0.6%)
631 failed (58.5%)
1 errored (0.1%)

I don't believe running anything else is relevant since I'm only moving the package location as per the issue tagged below. If this is not the case, please let me know, and I'll be happy to close more items from the checklist.

CLOSES #8450

@DatGreekChick DatGreekChick deleted the other/move-esm-to-dev-dependency branch July 18, 2019 23:38
@ryanhamley
Copy link
Contributor

I think you just need to change ^ back to ~. Newer versions of ESM break our test suite for some reason (see #8450) so we have to keep it locked to patch changes, hence the tilde semver range.

@mourner
Copy link
Member

mourner commented Jul 19, 2019

Also, please use Yarn instead of NPM so that Yarn.lock is updated properly as well.

@DatGreekChick
Copy link
Contributor Author

Ok will do and will either reopen or open a new PR. Thanks @ryanhamley and @mourner
However, when I changed it to a ~ I still got the same amount of integration tests failing locally. I'll try again and update you both.

@ryanhamley
Copy link
Contributor

Did you try with yarn install then yarn run test? It's a known issue that npm install will not likely break things.

@asheemmamoowala
Copy link
Contributor

@DatGreekChick can you share the failure result that you see locally?

@DatGreekChick DatGreekChick mentioned this pull request Aug 2, 2019
7 tasks
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.

esm dependency vunerability
4 participants