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

leverage import.meta.resolve to support import maps generation and accurate node_modules resolution when building #684

Closed
1 of 5 tasks
thescientist13 opened this issue Aug 4, 2021 · 2 comments · Fixed by #1326, #1341 or #1389
Assignees
Labels
alpha.0 alpha.1 breaking CLI enhancement Improve something existing (e.g. no docs, new APIs, etc) Plugins Greenwood Plugins v0.31.0
Milestone

Comments

@thescientist13
Copy link
Member

thescientist13 commented Aug 4, 2021

Type of Change

  • New Feature Request
  • Documentation / Website
  • Improvement / Suggestion
  • Bug
  • Other (please clarify below)

Summary

Coming out #674 for #570 which lets NodeJS do node_modules resolution for us by using require.resolve, one take away was that once NodeJS supports import.meta.resolve, we can use that instead.

Preview implementation for reference - https://github.com/ProjectEvergreen/greenwood/blob/v0.29.0/packages/cli/src/lib/node-modules-utils.js#L13

Details

In plugin-node-modules, we have a couple functions that are doing the work of finding entry points and building up an import map for the user, primarily for development purposes.

  • getPackageEntryPoint - as the name implies, it uses package.json to find the right entry point, but now that we are able to get this from require.resolve, do we sill need this at all?
  • walkModule - given an entry point, this walks through all the import and export statements to add to the overall importMap. I wonder though if export maps invalidates the need for this? In theory, wouldn't dependencies in package.json / main or exports fields be able to provide all the information we need?

In addition, we might not want to be hardcoding process.cwd() in walkPackageJson
https://github.com/ProjectEvergreen/greenwood/blob/master/packages/cli/src/plugins/resource/plugin-node-modules.js#L110

const walkPackageJson = async (packageJson = {}) => {

  for (const dependency of Object.keys(packageJson.dependencies || {})) {
    const dependencyPackageRootPath = path.join(process.cwd(), './node_modules', dependency);
    const dependencyPackageJsonPath = path.join(dependencyPackageRootPath, 'package.json');
    ...
  }
}
@thescientist13 thescientist13 added enhancement Improve something existing (e.g. no docs, new APIs, etc) chore unit testing, maintenance, etc CLI labels Aug 4, 2021
@thescientist13 thescientist13 added this to the 1.0 milestone Aug 4, 2021
@thescientist13 thescientist13 self-assigned this Aug 4, 2021
@thescientist13 thescientist13 changed the title use NodeJS require.resolve to support building of the import map use NodeJS (require.resolve) to support building of the import map Aug 4, 2021
@thescientist13 thescientist13 added the question Further information is requested label Aug 10, 2021
@thescientist13 thescientist13 removed the chore unit testing, maintenance, etc label Mar 13, 2022
@thescientist13 thescientist13 changed the title use NodeJS (require.resolve) to support building of the import map use NodeJS (import.meta.resolve) to support building of the import map Dec 16, 2023
@thescientist13 thescientist13 changed the title use NodeJS (import.meta.resolve) to support building of the import map leverage import.meta.resolve to support building of the import map during development Dec 19, 2023
@thescientist13
Copy link
Member Author

So far this is working nice, but seeing a minor issue regarding a package that has an empty main field in package.json, but can probably work around it
https://github.com/thescientist13/import-meta-resolve-demo

@thescientist13 thescientist13 moved this from 🔖 Ready to 🏗 In progress in [Greenwood] Phase 10 - Ecosystem Compat Nov 18, 2024
@thescientist13
Copy link
Member Author

Looks like this issue may help things in terms of getting to a package root, but will probably have to try / catch my through this in the meantime, and would help avoid the edge case with @types/trusted-types identified above

@thescientist13 thescientist13 changed the title leverage import.meta.resolve to support building of the import map during development leverage import.meta.resolve to support building import maps during development Nov 18, 2024
@thescientist13 thescientist13 changed the title leverage import.meta.resolve to support building import maps during development leverage import.meta.resolve to support import maps generation during development Nov 18, 2024
@thescientist13 thescientist13 changed the title leverage import.meta.resolve to support import maps generation during development leverage import.meta.resolve to support import maps generation and accurate node_modules resolution during development Nov 25, 2024
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in [Greenwood] Phase 10 - Ecosystem Compat Dec 2, 2024
@thescientist13 thescientist13 reopened this Dec 2, 2024
@thescientist13 thescientist13 moved this from ✅ Done to 🏗 In progress in [Greenwood] Phase 10 - Ecosystem Compat Dec 2, 2024
@thescientist13 thescientist13 changed the title leverage import.meta.resolve to support import maps generation and accurate node_modules resolution during development leverage import.meta.resolve to support import maps generation and accurate node_modules resolution when building Dec 2, 2024
@thescientist13 thescientist13 moved this from 🏗 In progress to 👀 In review in [Greenwood] Phase 10 - Ecosystem Compat Dec 9, 2024
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in [Greenwood] Phase 10 - Ecosystem Compat Dec 13, 2024
@thescientist13 thescientist13 added breaking Plugins Greenwood Plugins and removed question Further information is requested labels Dec 14, 2024
@thescientist13 thescientist13 linked a pull request Jan 19, 2025 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment