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

Undo import(module).start() convention #41

Merged
merged 1 commit into from
Dec 15, 2023
Merged

Conversation

mbland
Copy link
Owner

@mbland mbland commented Dec 15, 2023

This reverts part of commit 6ae34c2.

After further consideration, I realized that the .start() convention didn't really provide much use. I was thinking it would encourage thinking of window and document as arguments injected into the module. However, modules can be designed that way and tested directly without this idiom.

I did, however, take the liberty to filter out any <script type="module"> elements without a src attribute, for safety's sake.

This reverts part of commit 6ae34c2.

After further consideration, I realized that the .start() convention
didn't really provide much use. I was thinking it would encourage
thinking of window and document as arguments injected into the module.
However, modules can be designed that way and tested directly without
this idiom.

I did, however, take the liberty to filter out any <script
type="module"> elements without a `src` attribute, for safety's sake.
@mbland mbland self-assigned this Dec 15, 2023
@mbland mbland merged commit 2e9d2b6 into main Dec 15, 2023
3 checks passed
@mbland mbland deleted the undo-start-convention branch December 15, 2023 17:32
mbland added a commit that referenced this pull request Dec 15, 2023
After thinking about it, it doesn't make sense to instantiate a
component object with on internal state to invoke what could be a static
method. If a componenent needed to create a stateful object to manage
interactions, its static init() could take care of that.

Also removes the start() function from main.js, which should've been
removed in commit 366d9bc from #41.
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.

None yet

1 participant