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

Allow Require.delegate to bypass loadScript #137

Merged
merged 1 commit into from
Feb 12, 2019

Conversation

tejaede
Copy link
Contributor

@tejaede tejaede commented Nov 16, 2018

Provide hook to allow developer to take over script loading. This allows the developer an entry point to do a variety of things without interfering with mr's caching/mapping/redirect, etc.

Possible use cases:

  1. Application includes bundles that mr does not recognize. The developer can prevent an unnecessary HTTP request if a file is included in one of said bundles.

  2. Application scripts are stored on the file system and the browser has access. The developer can retrieve the file directly from the system and prevent an unnecessary HTTP request

@tejaede tejaede changed the title Allow Require.delegate to override loadScript Allow Require.delegate to bypass loadScript Nov 16, 2018
@tejaede
Copy link
Contributor Author

tejaede commented Nov 16, 2018

CC: @hthetiot @marchant

Proposed as solution to montagejs/montage#2000

Copy link
Collaborator

@cdebost cdebost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Would you rather be able to bypass loadIfNotPreloaded instead, with a Require.delegate.loadIfNotPreloaded method?

@tejaede
Copy link
Contributor Author

tejaede commented Feb 11, 2019

@cdebost It's an interesting thought. Perhaps they are not mutually exclusive?

The thing with loadIfNotPreloaded is it includes definition and preloaded which are not general purpose APIs. The user would have to implement that level of caching themselves.

@cdebost
Copy link
Collaborator

cdebost commented Feb 12, 2019

@tejaede True. I'm fine with merging this PR if you think it's ready to go, looks pretty harmless.

@tejaede
Copy link
Contributor Author

tejaede commented Feb 12, 2019

All good here! I'll merge!

@tejaede tejaede merged commit 864a8dc into montagejs:master Feb 12, 2019
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

2 participants