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

refactor!: do not copy JS lib to platform project #1203

Merged
merged 2 commits into from
Dec 1, 2021

Conversation

erisu
Copy link
Member

@erisu erisu commented Nov 22, 2021

Motivation and Context

The goal of this PR is to remove the pratice of copying the entire JS lib of cordova-ios into each generated platform project.

Description

  • create: do not copy our JS lib (or node_modules) to platform project
  • Move files around
    • bin/lib/create.js to lib/
    • bin/templates/scripts/cordova/lib to lib/
    • bin/templates/scripts/cordova/Api.js to lib/

After these changes, the cordova/ folder of a generated project only contains the bare minimum to maintain compatibility with existing cordova CLI versions (plus defaults.xml which IMHO should be moved in another PR). All of the remaining JS files in that folder require their dependencies via the cordova-ios module, so that needs to be resolvable somehow (which is the case for a platform project generated by the regular CLI workflow).

Testing

  • existing tests pass
  • I successfully created and built a cordova-ios project using cordova@10

@erisu erisu added this to the 7.0.0 milestone Nov 22, 2021
@erisu erisu force-pushed the refactor/do-not-copy-js-lib branch from df0cd6b to 9099962 Compare November 22, 2021 04:32
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2021

Codecov Report

Merging #1203 (2b1b031) into master (457bfa0) will increase coverage by 3.44%.
The diff coverage is 71.98%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1203      +/-   ##
==========================================
+ Coverage   75.09%   78.53%   +3.44%     
==========================================
  Files          13       15       +2     
  Lines        1658     1761     +103     
==========================================
+ Hits         1245     1383     +138     
+ Misses        413      378      -35     
Impacted Files Coverage Δ
lib/BridgingHeader.js 95.45% <ø> (ø)
lib/Podfile.js 75.12% <ø> (ø)
lib/PodsJson.js 95.04% <ø> (ø)
lib/build.js 66.04% <ø> (ø)
lib/check_reqs.js 47.05% <ø> (ø)
lib/listDevices.js 100.00% <ø> (ø)
lib/listEmulatorBuildTargets.js 96.77% <ø> (ø)
lib/listEmulatorImages.js 100.00% <ø> (ø)
lib/plugman/pluginHandlers.js 90.05% <ø> (ø)
lib/prepare.js 85.00% <ø> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 457bfa0...2b1b031. Read the comment docs.

Copy link
Contributor

@raphinesse raphinesse left a comment

Choose a reason for hiding this comment

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

🎉

tests/spec/create.spec.js Outdated Show resolved Hide resolved
@erisu erisu merged commit 07383c1 into apache:master Dec 1, 2021
@erisu erisu deleted the refactor/do-not-copy-js-lib branch December 1, 2021 03:46
gazben pushed a commit to apicore-engineering/cordova-ios that referenced this pull request Aug 26, 2022
Co-authored-by: Raphael von der Grün <raphinesse@gmail.com>
@yoldar
Copy link

yoldar commented Jul 31, 2023

@erisu Hi! Could you please tell me how to change from old method of getting /cordova/lib/projectFile.js to new one?
Here is code:
// try cordova 7.0 structure
const IosPlatformApi = require( path.join( iosPlatformPath(), "/cordova/Api" ) );
const projectFileApi = require( path.join( iosPlatformPath(), "/cordova/lib/projectFile.js" ) );
const locations = ( new IosPlatformApi() ).locations;
projectFile = projectFileApi.parse( locations );

@breautek
Copy link
Contributor

@erisu Hi! Could you please tell me how to change from old method of getting /cordova/lib/projectFile.js to new one? Here is code: // try cordova 7.0 structure const IosPlatformApi = require( path.join( iosPlatformPath(), "/cordova/Api" ) ); const projectFileApi = require( path.join( iosPlatformPath(), "/cordova/lib/projectFile.js" ) ); const locations = ( new IosPlatformApi() ).locations; projectFile = projectFileApi.parse( locations );

the lib files remain inside node_modules now, instead of being copied into the ios xcode project.

projectFile.js is not exposed in a public way, so if you need it, it would be great to learn why and maybe in the future we can add a public interface into it.

A hacky way getting access to projectFile.js is to simply require it: require('cordova-ios/lib/projectFile.js').

Note the old way that you posted might still be required if you intend to support cordova-ios 6 and earlier. Here is an example of how this plugin does it to support both versions.

@yoldar
Copy link

yoldar commented Jul 31, 2023

@breautek Thanks!

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.

None yet

5 participants