Skip to content

Fix require statements with plain template literals#7369

Merged
devongovett merged 5 commits into
parcel-bundler:v2from
wardpeet:fix/require-lit
Nov 28, 2021
Merged

Fix require statements with plain template literals#7369
devongovett merged 5 commits into
parcel-bundler:v2from
wardpeet:fix/require-lit

Conversation

@wardpeet

@wardpeet wardpeet commented Nov 26, 2021

Copy link
Copy Markdown
Contributor

↪️ Pull Request

Converts plain template literals into a string so dependency collector does find the dependency.

Fixes #7368
Fixes #5492

💻 Examples

export const startLogger = () => {
  require("./test");
  if (process.env.GATSBY_LOGGER.includes(`json`)) {
    // TODO move to dynamic imports
    require(`./logger/test.js`).initializeJSONLogger();
  } else if (process.env.GATSBY_LOGGER.includes(`yurnalist`)) {
    // TODO move to dynamic imports
    require(`./logger/test.js`).initializeYurnalistLogger();
  } else {
    // TODO move to dynamic imports
    require(`./logger/test.js`).initializeINKLogger();
  }
}

🚨 Test instructions

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@height

height Bot commented Nov 26, 2021

Copy link
Copy Markdown

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@wardpeet wardpeet changed the title Fix require statements with string literal Fix require statements with plain template literals Nov 26, 2021
Comment on lines +542 to +554
let mut arg = arg.clone();

// convert require(`./name`) to require("./name")
if let ast::Expr::Tpl(_tpl) = &*arg.expr {
if _tpl.quasis.len() == 1 && _tpl.exprs.is_empty() {
let tpl_str = &_tpl.quasis[0].raw;
arg.expr = Box::new(ast::Expr::Lit(ast::Lit::Str(ast::Str {
value: tpl_str.clone().value,
..tpl_str.clone()
})));
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not really familiar with Rust so if this code is not optimized or can be rewritten to something better, I'll take it :)

@mischnic

Copy link
Copy Markdown
Member

Can you add 2 new tests: one for the test you already changed (and revert that existing one), and one test that does something like this (which should still be ignored)

let x = 2;
require(`foo${x}`)

@wardpeet

Copy link
Copy Markdown
Contributor Author

Updated, found another piece that needed to be updated.

@devongovett devongovett left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! We could probably make a util function for this but I can do that separately.

@devongovett devongovett merged commit b08ef9c into parcel-bundler:v2 Nov 28, 2021
@devongovett

Copy link
Copy Markdown
Member

Here's that util: #7376

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.

Require statements with simple template literal does not get detected Recognize require call with template literals (but no placeholders)

3 participants