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

Blacklight javascript should be shipped as a module #2898

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Conversation

jcoyne
Copy link
Member

@jcoyne jcoyne commented Nov 15, 2022

Except for the sprockets use case which can't handle modules.

@jrochkind
Copy link
Member

Does this change how someone using, say, esbuild will use Blacklight JS? Should there be docs anywhere for how to do that?

(Or I guess the first prefatory question is, is it important to us to make sure it's even possible for someone using esbuild (or other jsbundling-rails approaches) to use Blacklight JS? I think a number of Blacklight users are using esbuild or would like to? I don't actually know anybody who wants to or plans to use rails "importmaps", although they may exist too!)

@jcoyne jcoyne force-pushed the esm-generate branch 2 times, most recently from 6b2db0e to 400e050 Compare November 15, 2022 18:14
@jrochkind
Copy link
Member

(and then another question -- is sprockets still going to be supported? Does this PR remove support for sprockets, or is it still there? I myself hope to no longer be using sprockets soon, but there was definitely concern expressed at blacklight summit about removing support for sprockets in Blacklight 8. I just want to draw attention to the issue, and ask a question about how the determination of what JS management methods to support should be made, and to be clear about whether a PR intends to change what JS management methods are supported).

@jcoyne
Copy link
Member Author

jcoyne commented Nov 15, 2022

Does this change how someone using, say, esbuild will use Blacklight JS? Should there be docs anywhere for how to do that?

I don't know. We don't have any tests for esbuild. We discussed this at a meeting and decided that we should favor the default rails ways of providing javascript, which for 7.0 is importmap.

(Or I guess the first prefatory question is, is it important to us to make sure it's even possible for someone using esbuild (or other jsbundling-rails approaches) to use Blacklight JS? I think a number of Blacklight users are using esbuild or would like to? I don't actually know anybody who wants to or plans to use rails "importmaps", although they may exist too!)

I'm not sure why you would jump to the conclusion that this PR changes any of this. What in particular here is concerning to you?

@jrochkind
Copy link
Member

OK, thanks for the answers!

I don't underestand this stuff enough to understand it, so appreciate the explicit confirmation!

I missed the meeting where it was decided to favor importmaps. If that's so, so be it!

@jcoyne , I am curious, do you plan to use importmaps in your app(s)? I hadn't previously heard of anyone who wanted to use them, all discussions I had been a part of I had only heard people say importmaps were not working out for them and they did not plan to use them. I personally do not want to use importmaps and would be disappointed if Blacklight made them or sprockets the only option -- at present, with or without tests, I believe people are using Blacklight with webpacker and/or only esbuild, so a change to that would be a change?

But ok, this is just my feedback, and I have not been at the meetings where it was apparently discussed, I'll leave it to y'all.

@jcoyne
Copy link
Member Author

jcoyne commented Nov 15, 2022

is sprockets still going to be supported? Does this PR remove support for sprockets, or is it still there?

Sprockets is still use and tested here.

I myself hope to no longer be using sprockets soon, but there was definitely concern expressed at blacklight summit about removing support for sprockets in Blacklight 8. I just want to draw attention to the issue, and ask a question about how the determination of what JS management methods to support should be made, and to be clear about whether a PR intends to change what JS management methods are supported

This doesn't change any of the supported methods.

@jcoyne jcoyne force-pushed the esm-generate branch 4 times, most recently from 5c302fb to beeeaf7 Compare November 16, 2022 14:17
@jcoyne
Copy link
Member Author

jcoyne commented Nov 16, 2022

@jrochkind

I am curious, do you plan to use importmaps in your app(s)? I hadn't previously heard of anyone who wanted to use them, all discussions I had been a part of I had only heard people say importmaps were not working out for them and they did not plan to use them. I personally do not want to use importmaps

Yes, I find they work great. I love not having any node dependencies or compilation steps to worry about.

@marlo-longley marlo-longley added this to the 8.0 milestone Dec 8, 2022
@tpendragon tpendragon merged commit 32dd31f into main Jan 23, 2023
@tpendragon tpendragon deleted the esm-generate branch January 23, 2023 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants