GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Updated packages.md and added --experimental-detect-module flag by shubherthi-mitra · Pull Request #51089 · nodejs/node

Conversation

@shubherthi-mitra

@shubherthi-mitra shubherthi-mitra commented Dec 7, 2023 •
edited by aduh95
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

added --experimental-detect-module flag in the docs to resolve #51057

Fixes: #51057

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@shubherthi-mitra
Update packages.md
818cca5
added --experimental-detect-module flag in the docs.
@nodejs-github-bot

nodejs-github-bot commented Dec 7, 2023

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to Node.js documentation. label Dec 7, 2023
@GeoffreyBooth

GeoffreyBooth commented Dec 7, 2023

Copy link
Copy Markdown
Member

It feels wrong to duplicate what we have in cli.md, though it seems we’re doing the same for --input-type. Perhaps both sections should just be links to the parallel sections in cli.md, rather than repeating the content and gradually getting out of sync.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@shubherthi-mitra

shubherthi-mitra commented Dec 8, 2023

Copy link
Copy Markdown
Contributor Author

It feels wrong to duplicate what we have in cli.md, though it seems we’re doing the same for --input-type. Perhaps both sections should just be links to the parallel sections in cli.md, rather than repeating the content and gradually getting out of sync.

@GeoffreyBooth I understand. But I did this after I asked here #51057 to fix the issue mentioned there. I'm new to github. My apology if any trouble caused.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@GeoffreyBooth

GeoffreyBooth commented Dec 8, 2023

Copy link
Copy Markdown
Member

@shubherthi-mitra you didn’t do anything wrong, thank you for the contribution! My question was more for @nodejs/loaders, if we want to repeat the info here like you’ve done in this PR or if we’d rather do it differently, such as by just mentioning the flag’s existence and linking to its primary documentation.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JakobJingleheimer

JakobJingleheimer commented Dec 8, 2023

Copy link
Copy Markdown
Member

just mentioning the flag’s existence and linking to its primary documentation

I think this (smaller surface area to update, less chance of divergence).

targos reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@aduh95

aduh95 commented Dec 8, 2023

Copy link
Copy Markdown
Contributor

I think what we want is to modify the paragraph added in https://github.com/nodejs/node/pull/50096/files#diff-71a7964a5c439435680952125e31e9dd4fca3a427982e6db86770fd0fbbfde63 to add a mention of the flag (and a link to cli.md).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@shubherthi-mitra
Updated packages.md
92aad94
Mentioned --experimental-detect-module flag and added its reference to cli.md
@shubherthi-mitra

shubherthi-mitra commented Dec 8, 2023

Copy link
Copy Markdown
Contributor Author

I think what we want is to modify the paragraph added in https://github.com/nodejs/node/pull/50096/files#diff-71a7964a5c439435680952125e31e9dd4fca3a427982e6db86770fd0fbbfde63 to add a mention of the flag (and a link to cli.md).

I did as you have said. Please check it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95
aduh95 reviewed Dec 8, 2023
View reviewed changes
Comment thread doc/api/packages.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

@shubherthi-mitra
Updated packages.md (more simplified)
df062e5
--experimental-detect-module flag and link to cli.md added.
aduh95
aduh95 reviewed Dec 8, 2023
View reviewed changes
Comment thread doc/api/packages.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

@shubherthi-mitra
Update packages.md added ref link
e2e556a
Mentioned --experimental-defect-module flag in doc and added ref link to cli.md
jasnell
jasnell approved these changes Dec 9, 2023
View reviewed changes
JakobJingleheimer
JakobJingleheimer reviewed Dec 9, 2023
View reviewed changes
Comment thread doc/api/packages.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

@shubherthi-mitra
Updated packages.md fixed a paragraph
b19e147
Simplified the paragraph under the heading ###Determining module system, mentioning --experimental-detect-module flag and reference to cli.md
JakobJingleheimer
JakobJingleheimer approved these changes Dec 9, 2023
View reviewed changes

@JakobJingleheimer JakobJingleheimer left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

shubherthi-mitra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
aduh95
aduh95 approved these changes Dec 9, 2023
View reviewed changes
@aduh95
aduh95 merged commit 37ba7a3 into nodejs:main Dec 9, 2023
@aduh95

aduh95 commented Dec 9, 2023

Copy link
Copy Markdown
Contributor

Landed in 37ba7a3, thanks for the contribution 🎉

shubherthi-mitra reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

RafaelGSS pushed a commit that referenced this pull request Dec 15, 2023
@shubherthi-mitra @RafaelGSS
doc: add note regarding --experimental-detect-module
637ffce
PR-URL: #51089
Fixes: #51057
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Dec 15, 2023
v21.5.0 proposal #51166
Merged
richardlau pushed a commit that referenced this pull request Mar 25, 2024
@shubherthi-mitra @richardlau
doc: add note regarding --experimental-detect-module
6c24155
PR-URL: #51089
Fixes: #51057
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@richardlau richardlau mentioned this pull request Mar 25, 2024
v20.12.0 proposal #52212
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@JakobJingleheimer JakobJingleheimer JakobJingleheimer approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

doc Issues and PRs related to Node.js documentation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Node can't recognize static import of ES module when theres's not explicit marker

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@shubherthi-mitra @nodejs-github-bot @GeoffreyBooth @JakobJingleheimer @aduh95 @jasnell
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.