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

Scoped packages for {N} 3.0 - @nativescript/core instead of tns-core-modules #4041

Open
NathanWalker opened this issue Apr 21, 2017 · 22 comments
Labels

Comments

@NathanWalker
Copy link
Contributor

@NathanWalker NathanWalker commented Apr 21, 2017

If possible, this would be nice:

import { Page, ObservableArray, isIOS, isAndroid } from '@nativescript/core';

I believe it would make for a clear and consistent approach to bundling and usage.

Cons to:

import { Page } from 'tns-core-modules/ui/page';
import { isIOS, isAndroid } from 'tns-core-modules/platforms';
import { ObservableArray } from 'tns-core-modules/data/observable-array';
  • tns is the command line utility and doesn't have great meaning here (too techy)
  • -core-modules, redundant, yes we know it's a module import and part of core
  • hard to remember paths/directories where classes are (even with auto-import mucks understanding of where things are simply)
  • not consistent with other modern approaches/ideas to bundling
  • too long and dashes are inconvenient to type
  • muddies up class files

I know 3.0 is close but organizing into a scoped package may not be troublesome to do and would be appreciated if considered since it appears there is already an attempt to scope the packages. If scoping is going to happen, please consider the above before going through 2 iterations of scoped naming.

Updated for clarity

tns-core-modules could then be published as:
@nativescript/core


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@NathanWalker NathanWalker changed the title Scoped packages for {N} 3.0 Scoped packages for {N} 3.0 - @nativescript instead of tns-core-modules Apr 21, 2017
@PeterStaev

This comment has been minimized.

Copy link
Contributor

@PeterStaev PeterStaev commented Apr 23, 2017

Actually this is not such a good idea. There is one substantial difference between scoped and the current submodules - You cant publish/install/update all packages inside a scope. So everything inside the tns-core-modules will have to be its own package, with its own version and will have to be published/installed separately. So in your package.json you will have to have something like:

{
  "@nativescript/application-settings": "rc",
  "@nativescript/application": "rc",
  "@nativescript/color": "rc",
  "@nativescript/connectivity": "rc",
  ....
  "@nativescript/ui": "rc",
  "@nativescript/utils": "rc",
  "@nativescript/xhr": "rc",
  "@nativescript/xml": "rc"
}

Then when the time comes to update, you will have to update all those version to the correct new one. Basically what you have to do now when a new angular version hits and you need to update your angular deps. And if you miss to upgrade some package it could cause problems.

And besides you do not need to type tns-core-modules for the packages at all. This is automatically handled by the runtime and by the TS config with the path resolution settings as described here. So you can have:

import { Page } from "ui/page"
import { Observable } from "data/observable"

And it will work.

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Apr 23, 2017

@PeterStaev this could be solved simply by:
@nativescript/core
This allows all other nativescript hosted packages to eventually follow same organization:
@nativescript/angular
It not only helps users organize their own package dependencies but strengthens the brand.

@NathanWalker NathanWalker changed the title Scoped packages for {N} 3.0 - @nativescript instead of tns-core-modules Scoped packages for {N} 3.0 - @nativescript/core instead of tns-core-modules Apr 23, 2017
@vakrilov

This comment has been minimized.

Copy link
Member

@vakrilov vakrilov commented Apr 24, 2017

@NathanWalker I agree with the points you made and IMO it looks better.

I'm not sure if we can afford to make another big change so close before the 3.0 lunch and in the current delicate state that we have many folks already working on migrating plugins and apps.

@PeterStaev

This comment has been minimized.

Copy link
Contributor

@PeterStaev PeterStaev commented Apr 24, 2017

Ok, having @nativescript/core, @nativescript/angular looks much better! Now if we can add also the plugins to the scope... 😄

@tsonevn tsonevn added the feature label Apr 24, 2017
@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Apr 24, 2017

@vakrilov @PeterStaev I wish I made suggestion a little earlier (I know timing is close - perhaps it's fairly simple to do) but yeah was just thinking since 3.0 was going to introduce breaking changes anyway, may be a perfect time to go ahead and do this - you could support both import paths as a way to ease transition with 3.0 - should be able to just publish under the scope and re-export everything in the target barrel.

@PanayotCankov

This comment has been minimized.

Copy link
Member

@PanayotCankov PanayotCankov commented Apr 25, 2017

Currently the tns-core-modules npm package is treated special. When you require "ui/core/view" the runtimes will actually check the "tns-core-modules" package. Publishing the modules in "@nativescript/core" can keep the apps somewhat backward compatible if the runtimes are modified to look in "@nativescript/core" instead of in "tns-core-modules".

I like the scopes, but I too think it is too late.

@vakrilov

This comment has been minimized.

Copy link
Member

@vakrilov vakrilov commented Apr 25, 2017

If there is a way to support both - we can introduce "@nativescript/core" in a minor release(ex 3.1, 3.2 etc.) and just promote it from then on.

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Apr 25, 2017

I'm in favor of 3.1 point release supporting this. 👍

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Feb 25, 2018

Core theme can be published as:

@nativescript/theme
@vakrilov

This comment has been minimized.

Copy link
Member

@vakrilov vakrilov commented Mar 1, 2018

Hey - I noticed some activity in twitter around this issue, so lets continue the discussion here.
@NathanWalker Maybe you can elaborate a bit more on what exactly the benefits of @nativescript/core will be in the light of bundling and schematics?

Concerns

Here are the concerns that I have with such change (re-publishing the tns-core-modules as @nativescript/core and deprecating tns-core-modules):

  1. It will be a breaking change and people will be required to change their imports and package deps.
  2. What about plugins? Consider the situation that you have switched to using @nativescript/core, but your plugin dependencies are still using tns-core-modules. You will end up with two versions of the NativeScript modules in your app.
  3. As @PanayotCankov noted there is some special logic in the runtimes that handles tns-core-modules. There might be some additional work related to that as well.
  4. There might be a period of confusion during the migration period as there will be yet another way to import Page

Possible Alternatives

@nativescript/core as just a Proxy package

One idea that might address some of the points is to have a @nativescript/core package that just has a dependency on the original tns-core-modules and re-exports the most commonly used classes. It should handle 1., 2. and 3.. However, 4. will be still a concern and additionally we will have to support/publish/maintain another package which might be just an overhead.

Have an index.ts files in the framework and reexport most commonly used classes there. For example you will be able to import Page, Label, Button from tns-core-modules/ui. This should address the hard to remember directories where classes are point from the original post.

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Jul 10, 2018

@vakrilov I'll revive this. I'm with the proxy package - in fact this is something we could introduce in xplat rather soon and could begin trying this out.

No matter what it would be a gradual thing but would be great to have this already available so could introduce the proxy and allow others to start using it and eventually it would just become the norm/standard. I would start using it immediately for sure.

@vakrilov

This comment has been minimized.

Copy link
Member

@vakrilov vakrilov commented Jul 15, 2018

Hey @NathanWalker - let's reiterate again on what the benefits of this will be.

  1. Naming. @nativescript/core looks better than tns-core-modules. Having angular and theme inside @nativescript scope is also aesthetically appealing.
  2. Having to import different UI elements form specific places inside the tns-core-modules (ex. 'tns-core-modules/ui/page'). We can solve that, with an index.ts in the root of the package which exports the most commonly used classes.

Is there something else I'm missing. You mentioned bundling at the original post but I'm not sure how that is related? Also is there something xplat-specific that will benefit form having the @nativescript scope?

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Jul 15, 2018

That hits the nail on the head 👍 Nothing xplat specific (other than allowing more helpful and reliable schematics to auto update user's apps with various best practices, etc - Consistent and simpler import packaging helps make things like that possible).

Whatever is done here let's be gradual so the old imports would be supported for another 1-2 years likely. However the new import pathing could start showing up - we would use it as default in our xplat tools as a way to introduce it to the angular workspace crowd. Slowly overtime the hope would be for it to make more sense to broader audience and intuitively folks would just use it. I could help document 1 single clear doc page which would just show every package and how it maps to the simpler import pathing for those that want to start using it. Current docs wouldn't need to be updated for awhile and again could be very gradual since both forms would work just fine.

@PeterStaev

This comment has been minimized.

Copy link
Contributor

@PeterStaev PeterStaev commented Feb 20, 2019

So I want to revive this with the recent "depreciation" of the short import paths in 5.2 and requiring users to type the full path "tns-core-modules". The current import paths are already a bit long and requiring to type an additional 31 symbols in front I think is a bit too much.

Before fully removing short import paths this should be considered with high priority and shortening the import paths altogether. What was discussed above sounds really good - like importing UI modules from a single point @natviescript/ui.

Hoping the core team will re-evaluate this issue and we will have this implemented at the point where the short import paths will be completely removed 🙏

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Feb 23, 2019

Reference to this working wonderfully well today within Nx + xplat workspaces:
https://nstudio.io/blog/say-hello-to-scoped-nativescript/

@edusperoni

This comment has been minimized.

Copy link
Contributor

@edusperoni edusperoni commented May 31, 2019

Maybe this could be done in the following way:

  1. Introduce @nativescript scoped packages as a meta package that depends on tns-core-modules
  2. Provide a migration tool that searches for imports from tns-core-modules and replaces with the equivalent @nativescript import. Similar to the RxJS 5->6 migration tool (maybe we could get a package like tns-core-modules-compat so that modules that can't be migrated could be imported from the compat package). Since short imports are already deprecated, it's easier to find tns-core-modules references and update them with a migration tool. This could even be done with a compatibility beforePrepare hook.
  3. Since a lot of plugins have to be migrated to androidx, introduce this migration along with it

As we're also using webpack by default now, we can also replace imports with new webpack.NormalModuleReplacementPlugin(/^tns-core-modules/ui/.*$/, "@nativescript/ui"). This would also force people who haven't migrated to long imports to do it.

In the future, everything can be migrated to @nativescript packages and then shift tns-core-modules so it's a compatibility package (which could be eventually removed in NS 7 or 8).

@PeterStaev

This comment has been minimized.

Copy link
Contributor

@PeterStaev PeterStaev commented Jun 1, 2019

Actually RxJS compat is a bit the opposite - the new version uses the new import paths, and the -compat translates OLD to NEW import paths. So it is much better to add the suggested @nativescript packages in 6.0 and provide a compat module that translates tns-core-modules/* to @nativescript/*. This way short import paths will not be valid and will force plugin authors (like me) to upgrade to the new @nativescript/* modules 😄

Considering the changes coming with 6.0 and that authors would probably have to make required changes for AndroidX support I think it is the perfect time to add this.

@NathanaelA

This comment has been minimized.

Copy link
Contributor

@NathanaelA NathanaelA commented Jun 1, 2019

I agree I think going to @nativescript/* makes sense in an already breaking change version. Since you are already eliminating the legacy build model; this is a much much much less of a breaking change; than that. Might as well break everything. 😀

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Jun 11, 2019

Nothing would break with this approach just to be clear. This would simply provide a way forward with a simpler packaging approach which folks could start using today. Over time my guess is this would become the norm and only then would deprecating the older package naming could occur. Talking years, but having this available today (alongside the other) is really a no brainer to me.

Ultimately this would likely simplify teaching the framework and developer usage. Sounds like small thing but package location and usage of the framework has big impact on learning and retention as well as developer joy :)

Link to download the reexports we already use in our projects which makes this work today (for anything missing just expand if needed): https://drive.google.com/file/d/1AUPSxkwQIIvdK2ohoekuuts85ZZ4i2-l/view?usp=sharing

@shirakaba

This comment has been minimized.

Copy link

@shirakaba shirakaba commented Jun 12, 2019

Nothing would break with this approach just to be clear.

I think this is constantly being missed. I don’t believe Nathan is suggesting reorganising the layout of folders in tns-core-modules at all. He is merely suggesting the creation of a few re-exporting (barrel) modules under the @nativescript namespace. This wouldn’t introduce any breaking changes in tns-core-modules at all, and wouldn’t change the way that it’s published. Correct me if I’m wrong!

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Jun 12, 2019

@shirakaba 100% correct. No breaking change, no reorg, both ways work for long time. Folks can use either they like, can mix, plugins continue to work for long long time.

@NathanWalker

This comment has been minimized.

Copy link
Contributor Author

@NathanWalker NathanWalker commented Jun 13, 2019

Just for full clarity/context on this issue - public poll resulted in 25-0 in favor of wanting this:
https://twitter.com/wwwalkerrun/status/1138582335576715265

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.