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

[Feature]: Add .js file extension to built output to improve native ESM support #30634

Open
1 task done
spmonahan opened this issue Feb 27, 2024 · 5 comments · Fixed by #30770 · May be fixed by #30633 or #30620
Open
1 task done

[Feature]: Add .js file extension to built output to improve native ESM support #30634

spmonahan opened this issue Feb 27, 2024 · 5 comments · Fixed by #30770 · May be fixed by #30633 or #30620

Comments

@spmonahan
Copy link
Contributor

Library

React Components / v9 (@fluentui/react-components)

Describe the feature that you would like added

I want Fluent's ESM output (CJS can go either way I think) to include the .js extension for all relative JS imports. For example,

// Source Typescript
import { feature } from './file';

// Output ESM Javascript
import { feature } from './file.js';

This update will better allow Fluent's ESM output to be loaded directly in the browser (import maps will still be required to resolve bare package imports but that's no problem).

Ideally we should do this without changing source code and leverage something like SWC's @swc/plugin-transform-imports (which transforms exports in addition to imports despite the name).

Have you discussed this feature with our team

@Hotell @chrisdholt

Additional context

Currently the ESM output from Fluent is "extensionless" which prevents the ESM output from working correctly in a browser.

// Example output of current output
import { FluentProvider } from '@fluentui/react-components'; // bare module
export { something } from './somewhere'; // bare JS file

// Example of what this needs to be for native ESM support
import { FluentProvider { from '@fluentui/react-components;
export { something } from './somewhere.js';

In this example there is a "bare module" import where code is imported from a package name (that lacks a .js extension) and a "bare JS file" import. The latter is typically used within a package to import other JS files using relative paths.

We can address "bare modules" with importmaps, allowing import $whatever from 'some-module-without-an-extension' to resolve correctly but importmaps cannot, effectively, handle the internal bare JS imports. To handle the bare JS imports we must update the output to include the .js extension.

I say effectively because it is possible to generate an importmap for all the base JS imports within a package but such an importmap would be quite large and then require end users to correctly merge it with any other importmaps they are using. Technically possible but not a good solution :)

Related issues

Validations

  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Priority

Normal

@Hotell
Copy link
Contributor

Hotell commented Mar 1, 2024

@miroslavstastny I'm re-assigning this to v-build as we will be shipping it

@Hotell
Copy link
Contributor

Hotell commented Mar 5, 2024

Update:

Previously I mentioned that in order to make this work, our packages need to be either type:module or ship .mjs under exports#import mapping.

I explored this further and possiblity to add "just .js extensions" is also possible but has some issues which I'll expand on in 1st option description.

Approach options:

1. adding explicit .js extension on build time

  • NodeJS: we use exports#node mapping within our packages so no matter if we ship native ESM, if our packages are loaded in node, it will always consume commonjs outputs ATM
  • Browser: if one want's use native ESM, package modules need to be compliant with ESM format - all imports/exports need to specify extension. Export maps won't work with raw native ESM, so if a package includes re-exports from another package via bare import/export specifier (export {Hello} from '@fluentui/hello';) user is forced to provide import-maps, otherwise it will throw error

👁️ This has following gotchas:

We need to enforce that relative imports always point to existing physical file no matter what module loader is in place (node vs browser).

🧪 Example:

// @filename src/index.ts
export {Hi} from './hello'

// @filename src/hello/index.ts
export {Hi} from './hi'

⬇️ swc transpile ( + @swc/plugin-transform-imports ) ⬇️

// @filename lib/index.js
// 🚨🚨🚨 this is invalid path - will throw in all environements
export {Hi} from './hello.js'

// @filename src/hello/index.js
export {Hi} from './hi.js'

Rollup handles this as expected

// @filename src/index.ts
export {Hi} from './hello'

// @filename src/hello/index.ts
export {Hi} from './hi'

⬇️ rollup ⬇️

// @filename lib/index.js
// ✅ ✅ ✅ unwraps the import path to it's source 
export {Hi} from './hello/hi.js'

// @filename src/hello/index.js - wont be created
// ✅ ✅ ✅ nested barrel files are completely removed from output as their are not necessary 

What are our implementation options:

2. declaring package as ESM native via type:module pragma

  • use .js extension within imports within package scope + everything mentioned in 1) applies

3. shipping all ESM files with .mjs extension

By default in ESM native environments, if a package.json has no type specified it fallbacks to type:commonjs by default (our packages situation).

While we have export maps in place, they are invalid for native ESM loader ( it works solely for build tools with current output ).

In order to support native ESM loader without making the package native ESM, all files that are accessed via exports#import need to:

  • have .mjs extension
  • use .js extension within imports within package scope

Implementation requirements:

NOTE: solutions need changes to webpack ( because storybook, cypress use webpack v4)

1. adding explicit .js extension on build time

  • build time only (won't work reliably with current setup, see 1st Approach options: above )

2. declaring package as ESM native via type:module pragma

  • tooling changes
    • just-script patch needed
    • jest moduleMapper hacks / '^(\\.{1,2}/.*)\\.js$': '$1',
  • source changes
    • TypeScript: moduleResolution": "NodeNext" / modifying all imports in .ts files to include .js extension

3. shipping all ESM files with .mjs extension

@spmonahan
Copy link
Contributor Author

Re-opening since we had to revert this in #30803

@yuntian001
Copy link

I need it. When do you plan to fix it

@AnujAgrawal-1
Copy link

AnujAgrawal-1 commented Jul 15, 2024

@spmonahan @Hotell Any update on above issue as i am also getting same issue
SyntaxError: Unexpected token 'export' at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1728:14) at Object.<anonymous> (node_modules/@fluentui/react-provider/lib-commonjs/components/FluentProvider/renderFluentProvider.js:15:20)

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