-
Notifications
You must be signed in to change notification settings - Fork 7
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
use @pika/pack to build ESM, UMD #21
Conversation
Wow thanks a lot! I'm on vacation this weekend so I'll have to look into it next week. I'll likely tweak some stuff, but a lot of thanks for getting it started. |
@matthewp I would love to include a link to custom-attributes in a "packages already using @pika/pack" section of the announcement post. If you have any interest lmk! |
Yeah, that would be great! Reviewing this now.. |
@@ -59,7 +59,7 @@ class BgColor { | |||
} | |||
} | |||
|
|||
customAttributes.define('bg-color', BgColor); | |||
customAttributes.default.define('bg-color', BgColor); |
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.
Hm, yeah I don't want to have to do this... Will look for a way around.
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, curious if you have any thoughts. Moving to ESM in your examples would definitely move people in that direction, but I didn’t see anything in rollup that lets you define your UMD module as an actual function
I'm going to go ahead and merge and then cleanup the |
@FredKSchott is it possibly to specify a different output file for the web build? I would like to not make users go deep with |
Since you publish the pkg directory directly, technically it will just be https://unpkg.com/custom-attributes/dist-web/index.js , but I do get your point about length. Would it be enough to let you add the “unpkg” entrypoint so that you can just do https://unpkg.com/custom-attributes ? |
Wouldn't that result in redirects though? I guess I'd be ok with Is there a way to get pika/pack to copy the source files over into the pkg/ folder? |
For clarity what I'd really like is:
Does adding |
Hmm, okay let me think about that a bit. The Essentially what you're requesting is cosmetic ( fwiw, since your package only contains |
Looks like the README is outdated: <script src="node_modules/custom-attributes/attr.js" defer></script> Maybe that should be <script src="node_modules/custom-attributes/dist-umd/index.js" defer></script> ? |
What about calling |
That's not a bad idea. We'll probably need to make a change in Pika anyway once Node gets ESM support, or decide that dist-node & dist-web can still be different even if both are ESM. |
Are these If so, then I think what makes sense to me, as far as module formats is something like |
Turns out this is a breaking change. In previous code of mine I was able to do the following to register the global import 'custom-attributes/attr.js' So I could build my app with a bundler like Webpack or Meteor, and it would cause the global to be defined, so I could use it like a polyfill. But now I'm installing from git (I have a fork of custom-attributes), and I'm importing it like this: import 'custom-attributes/pkg/dist-umd' I've committed the But due to the environment that I'm in (Meteor 1.6, which uses CommonJS as the underlying module system that ES modules compile down to), the UMD module sees the |
Can you |
Looks like Meteor reads the |
|
I'm guessing that If so, perhaps EDIT: Heck, who knows, maybe someone is still using RequireJS in a project. Might as well include |
@pika/pack: package.json config
Build Summary
Summary of Changes
attr.js
: Now handled by @pika/pack!global.js
: Now handled by @pika/pack!index.js
®istry.js
intosrc/
directorywindow.customAttributes
, the default registry has to live as a property of that namespace ( atcustomAttributes.default
or something similar).import customAttributes from 'custom-attributes'