-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
Create official loader
module API.
#118
Conversation
Provides an internal `loader` module with the following named exports: * `getIds` - Returns an array containing the `id`'s of all registered modules. * `has` - Returns `true` or `false` if a given module exists. * `define` - Defines a new module. * `require` - Requires a module (optionally relative to the current module). * `resolve` - Returns the expanded module name given a relative path.
how does this differ from |
@stefanpenner - |
IMHO, we should probably eventually deprecate I'm not suggesting any deprecations just yet, just pondering.... |
Also added some documentation for the new API. |
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.
Overall this seems good to me, just one comment about naming (not super important).
/* | ||
Return the list of module id's that are present in the registry | ||
*/ | ||
getIds(): string[]; |
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.
May just be me, but for some reason, I do not think id
when I think of the identifier for a module. Usually path
or name
instead. When I first saw getIds()
I had no idea what information it was actually returning.
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.
this is the unique id, usually name is relative or not where as this is registry specific.
https://whatwg.github.io/loader/#loader-import if you look here name is used for APIs like import() but the registry uses keys.
Would getKeys() be ok with you?
I hate naming things.
Where are we on this? I believe that all of the comments here have been addressed. What other blockers are there? |
@stefanpenner / @krisselden - Are y'all ok with this? Any changes needed? |
I'm not terribly thrilled to:
Is there a reason we can't just add more things to require? |
We can handle folks overriding via
I'm happy to deprecate
A couple things that pop out at me:
|
I don't think we can, as I still feel we should just expand the API on |
closing due to inactivity |
Provides an internal
loader
module with the following named exports:getIds
- Returns an array containing theid
's of all registeredmodules.
has
- Returnstrue
orfalse
if a given module exists.define
- Defines a new module.require
- Requires a module (optionally relative to the current module).resolve
- Returns the expanded module name given a relative path.Closes #82.