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

core(build): add LH_ROOT support to inline-fs #13278

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Conversation

brendankenny
Copy link
Member

part of #13231

lets inline-fs replace LH_ROOT with the value of LH_ROOT. I expected this PR to have more but the locale replacement stuff had to move to #13275

Support is hardcoded into inline-fs rather than plumbing through options because it's only one custom thing and we've never really needed anything else out of brfs. If the number of identifiers (or whatever) needing to be replaced grows in the future we can always add it later (just say no to magic bundler syntax though :).

Next I was planning on adding source map and watch file support, but I believe #12771 should be good to go after this PR @connorjclark?

@brendankenny brendankenny requested a review from a team as a code owner October 28, 2021 20:52
@brendankenny brendankenny requested review from connorjclark and removed request for a team October 28, 2021 20:52
@google-cla google-cla bot added the cla: yes label Oct 28, 2021
@@ -280,6 +282,9 @@ function collapseToStringLiteral(node, filepath) {
return path.dirname(filepath);
} else if (node.name === '__filename') {
return filepath;
} else if (node.name === 'LH_ROOT') {
// Note: hardcoded for LH. Could be be set via inline-fs options instead.
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you want to go the extra mile and publish this plugin to npm eventually? I bet others would like to use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

not today but I left the comment so it would be clear there's not much to do if we ever do want to.

Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

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

Next I was planning on adding source map and watch file support, but I believe #12771 should be good to go after this PR @connorjclark?

Yup, this should be the last piece needed to finish up that PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants