-
-
Notifications
You must be signed in to change notification settings - Fork 404
Simplified imports #946
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
base: main
Are you sure you want to change the base?
Simplified imports #946
Conversation
|
|
||
| **`<my-app>/components/modal.gjs`** | ||
| ```js | ||
| import { Component, on, or, tracked } from 'vidu-web'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't want to do this, but I think I'd prefer something like:
import Component from '@glimmer/component';
import { on, or } from '@glimmer/template';
import { tracked } from '@starbeam/ember';where,
@glimmer/template has:
This combines @glint/template, ember-truth-helpers
- the types from
@glint/templateModifierLikeHelperLikeComponentLikeInvoke(currently private, but is the only way ember-resources is glint compatible 😅)
- eq
- notEq
- gte
- gt
- or
- and
- nor
- xor
- nand
- lt
- lte
- not
- get
- concat
- uniqueId
- page-title
and,
@starbeam/ember has:
this combines parts of @glimmer/tracking, tracked-built-ins, ember-resources, and ember-modifier (because with the right manage wiring, modifiers are resources)
- cell
- resource
@tracked@cached- ReactiveMap
- ReactiveSet
- ReactiveWeakMap
- ReactiveWeakSet
and these are
built in
and no longer (and many already didn't) need importing:
onfnhasharrayhasBlockhasBlockParamshelper(for currying)modifier(for currying)component(for currying)in-elementlogdebugger
These should be deprecated / removed, imo
- action
- input
- link-to
- mount
- mut
- textarea
- unbound
Packages no longer needed (if all the above is done)
@ember/modifier@ember/helper@glimmer/tracking@glint/templatetracked-built-insember-resourcesember-truth-helpers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I'm not a fan of the original proposal. I prefer clear import paths instead of importing everything from a single namespace to make it a little bit easier. With proper tooling support there shouldn't be a difference anyway.
I do think revisiting and streamlining the imports, as you suggest here, makes sense but that seems like a whole different discussion compared to what's suggested in the RFC? 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see how importing from the originating source package can be considered clearer, but it brings with it downside of introducing multiple concepts like Glimmer, Starbeam etc. Every-time you need to import something, there's the small mental tax of trying to remember which package it comes from.
I don't have a strong opinion on this, but from the point of view of someone new to Ember, I would think that importing common Ember constructs from an easy to remember place would simplify things greatly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can incrementally get there, at the most basic, we should reorganize the tools anyway.
Once we have vite by default, and can ensure that tree-shaking works, we could provide an import { on, fn, eq, tracked, etc } from '@ember/meta';
but I don't think it makes sense to strive for something like that unless we can make sure that unused functions are not shipped in folks apps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but it brings with it downside of introducing multiple concepts like Glimmer, Starbeam etc
fwiw, all other frameworks have this split, so at the very least, we meet the status quo.
- nextjs has a ton of imports
- svelte has very few
- nuxt seems to all be globals
- remix has very few
Given the above suggestion, I think we'd have only 3 main concepts / imports for the whole framework (given that we align with all of Polaris' goals, as presented in the emberconf 2023 keynote)
- a component (a feature / refactoring boundary for a collection of "things")
- the reactive primitives and utilities (starbeam, works in JS and template)
- the template ulities (glimmer, needed pre-expression syntax (super handwave here, not anywhere near polaris timeline))
There are a lot of modules not mentioned, but I think if go hard on Polaris, we won't need most of our existing imports.
…t the next number was going to be already
|
Discussed at RFC review meeting, provided some general feedback to author with ideas. This is not concrete enough yet to advance. |
|
I actually do like having all these imports. Yep, sometimes its a lot, but that makes all these dependencies visible (My maximum is 27 imports The problem yet, is to remember where these imports are coming from. Basically, each and everyone I need to re-lookup. It's hard to tell, whether this is part of For what would help is Identity to these. Having glimmer to be "low-level" doesn't really match reality anymore, as nobody really can't tell what "low-level" means. Asking (core) members, where a certain thing should live, let's say For whatever the identity of |
for
Exactly why I think it's worth exploring a unification of the imports.
beyond that you have other userland implementations of things, and that's where you get multiple imports (
I think one thing we've not been great at is defining which For folks that want reduced imports, it's perfectly fine, and codemoddable (from an ecosystem standpoint) to create re-exports, as @GavinJoyce has done. It makes sense because we currently don't have good auto-import / intellisense for the imports. We need to PR to glint to enable this. If folks type less, then the burden of typing is felt less. Folks could choose to abstract their imports in their organization (hopefully only if they have good documentation or a small well-sync'd team): there is no risk here (other than documentation) -- folks choosing to do this in large organizations may want to add a lint to auto-correct the direct imports to their app-local re-exports. |
From: #945
Discord convo: https://discord.com/channels/480462759797063690/518154533143183377/1140567005984989264
Super thanks to @GavinJoyce for getting this started 🎉
Current status:
What could help in the mean time: