Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Windows Platform Release Preparation (Cordova 9) #319

Merged
merged 6 commits into from
Jan 17, 2019

Conversation

erisu
Copy link
Member

@erisu erisu commented Jan 9, 2019

Platforms affected

windows

What does this PR do?

This PR contains final preparations for the Cordova 9 release goals.

See Cordova 9 Release Plan.

  • Bumped cordova-common@^3.1.0
  • Bumped ESLint dependencies with correction
    • eslint@^5.12.0
    • eslint-config-semistandard@^13.0.0
    • eslint-config-standard@^12.0.0
    • eslint-plugin-import@^2.14.0
    • eslint-plugin-node@^8.0.1
    • eslint-plugin-promise@^4.0.1
    • eslint-plugin-standard@^4.0.0
  • Bumped Dev Dependencies & Pinned Jasmine at 3.1.0
    • jasmine@3.1.0
    • rewire@^4.0.1
  • Updated Dependencies
    • nopt@^4.0.1
  • Replace istanbul dependency with nyc@^13.1.0

What testing has been done on this change?

  • npm run eslint
  • npm run cover (unit testing)
  • Travis CI
  • AppVeyor
  • Platform Add & Prepare
    $ npx cordova@nightly create androidTest com.foobar.androidTest
    $ npx cordova@nightly platform add github:erisu/cordova-windows\#cordova9-prep
    $ npx cordova@nightly prepare windows
    

@@ -50,7 +50,9 @@ module.exports = function patch (platform) {
// 1. Find start page path in appxmanifest
var AppxManifest = require('./lib/AppxManifest');
var appxmanifest = AppxManifest.get(path.join(__dirname, '..', appxmanifestMap[platform]));
var startPage = url.parse(appxmanifest.getApplication().getStartPage());

// @todo Use 'url.URL' constructor instead
Copy link
Member

Choose a reason for hiding this comment

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

?

Copy link
Member Author

@erisu erisu Jan 9, 2019

Choose a reason for hiding this comment

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

ESLint Notice

53:21  error  'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead  node/no-deprecated-api

Choose a reason for hiding this comment

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

I would favor fixing this one before making the new release, if possible.

Copy link
Member Author

@erisu erisu Jan 10, 2019

Choose a reason for hiding this comment

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

The ESLint notice is for Node 11+, when it becomes deprecated and which we do not support yet.

Per documentations, changes would be as follows:

var startPage = new URL(appxmanifest.getApplication().getStartPage());

or

var startPage = new url.URL(appxmanifest.getApplication().getStartPage());

appxmanifest.getApplication().getStartPage() pulls the start page from package.*.appxmanifest, which is www/index.html.

Here is the output of the current usage url.parse("www/index.html"):

Url {
  protocol: null,
  slashes: null,
  auth: null,
  host: null,
  port: null,
  hostname: null,
  hash: null,
  search: null,
  query: null,
  pathname: 'www/index.html',
  path: 'www/index.html',
  href: 'www/index.html' }

Now, let's take a look at what new URL("www/index.html") and new url.URL("www/index.html") prints out with our supported Node versions.

Node 6
with new URL("www/index.html"):

ReferenceError: URL is not defined

with new url.URL("www/index.html"):

TypeError: Invalid URL: www/index.html

Node 8
new URL("www/index.html"):

ReferenceError: URL is not defined

new url.URL("www/index.html"):

TypeError [ERR_INVALID_URL]: Invalid URL: www/index.html
  at onParseError (internal/url.js:219:17)
  at parse (internal/url.js:228:3)
  at new URL (internal/url.js:311:5)

Node 10
new URL("www/index.html")
and
new url.URL("www/index.html"):

{ TypeError [ERR_INVALID_URL]: Invalid URL: www/index.html
  at onParseError (internal/url.js:240:17)
  at parse (internal/url.js:249:3)
  at new URL (internal/url.js:324:5) input: 'www/index.html' }

Since we do not support Node 11 currently, I hope that this can be postponed till after the release.

@erisu erisu mentioned this pull request Jan 10, 2019
33 tasks
Copy link
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

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

👍 from me

@erisu erisu merged commit fb62d84 into apache:master Jan 17, 2019
@erisu erisu deleted the cordova9-prep branch April 4, 2019 06:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants