Skip to content
This repository has been archived by the owner on Aug 27, 2018. It is now read-only.

package.json: Change main to esnext #174

Merged
merged 3 commits into from
Nov 2, 2017

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Aug 3, 2017

To better denote we're dealing with untranspiled code.

Companion Calypso PR: Automattic/wp-calypso#16870

To provide a bit of an explanation -- esnext is a custom field.

  • We'll specify it as a possible entry point for webpack to watch out for here.
  • OTOH, we use it (as opposed to main) as a flag to indicate that this is a package's entrypoint for untranspiled code here.

This is a strategy found at http://2ality.com/2017/06/pkg-esnext.html.

Suggested order:

package.json Outdated
@@ -2,7 +2,7 @@
"name": "notifications-panel",
"version": "1.2.1",
Copy link
Member

Choose a reason for hiding this comment

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

this is probably worth a major version bump, no? since software already using this would have to update in order not to break?

ockham and others added 2 commits October 30, 2017 23:36
To better denote we're dealing with untranspiled code. See http://2ality.com/2017/06/pkg-esnext.html
We previously used `main` inside of `package.json` to define where to
pull in untranspiled code into the project and this change starts using
the `esnext` property as seen in http://2ality.com/2017/06/pkg-esnext.html

That means (to me at least) that any software which has been using this
will have to make changes in order to not break when updating from the
previous versions when their build systems looked for the `main` property.
@dmsnell dmsnell force-pushed the update/package-json-main-to-module branch from 1bdb93a to 48348a9 Compare October 31, 2017 03:38
@dmsnell
Copy link
Member

dmsnell commented Oct 31, 2017

@gwwar @kwight I have updated this with a major version bump (see the commit description).

Could you two consider this change and indicate if you have any hesitation or questions? This is a change to help @ockham improve Calypso's build process by automatically inferring which node_modules can be included as-is without having to import the already-transpiled builds (which would end up speeding up our build and saving on build-size).

Copy link
Member

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

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

Pending review from others

@kwight
Copy link
Contributor

kwight commented Oct 31, 2017

@ockham do you have a strict timeline for this? We've moved to scheduled releases for notifications-panel (p3fqKv-5V6-p2), so this could be part of the next beta on Nov 8th, with production release Nov 22nd; would that work for you?

@gwwar
Copy link
Contributor

gwwar commented Oct 31, 2017

To add to that Calypso gets the beta release so unless it's really urgent, it'd be slated for Nov 8.

@ockham
Copy link
Contributor Author

ockham commented Nov 1, 2017

No strict/urgent timeline, but would totally appreciate this being merged!

@gwwar
Copy link
Contributor

gwwar commented Nov 1, 2017

Thanks @ockham we'll try to test/merge this soon, though for now we'll stick with the Nov 8 version bump.

kwight added a commit that referenced this pull request Nov 2, 2017
THis PR has been replicated on the `react16` branch as v2.1.0. This
minor version bump will allow either of the two Calypso-related PRs to
land first.

Related:
* Automattic/wp-calypso#16870
* Automattic/wp-calypso#19083
@kwight kwight merged commit 855fc30 into master Nov 2, 2017
@kwight kwight deleted the update/package-json-main-to-module branch November 2, 2017 22:49
@kwight
Copy link
Contributor

kwight commented Nov 2, 2017

I've committed these changes both to master (by merging this PR with a minor bump to 1.3.0) and to the react16 branch (with a bump to 2.1.0) in 011fc14.

@ockham
Copy link
Contributor Author

ockham commented Nov 29, 2017

D'oh, this won't work until we merge Automattic/wp-calypso#19698

Quoting Automattic/wp-calypso#19723 (comment):

To fix, we can:

kwight added a commit that referenced this pull request Dec 5, 2017
centered images into Automattic/wp-calypso.
kwight added a commit that referenced this pull request Dec 5, 2017
…and centred images into Automattic/wp-calypso. (#199)
kwight added a commit that referenced this pull request Dec 5, 2017
…k icons and centred images into Automattic/wp-calypso. (#199)"

This reverts commit 2f37504.
kwight added a commit that referenced this pull request Dec 5, 2017
…k icons and centred images into Automattic/wp-calypso. (#199)" (#200)

This reverts commit 2f37504.
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.

4 participants