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

Expo/React Native: Unable to resolve sub path import alias #1311

Closed
Beachman4 opened this issue Oct 24, 2024 · 14 comments
Closed

Expo/React Native: Unable to resolve sub path import alias #1311

Beachman4 opened this issue Oct 24, 2024 · 14 comments

Comments

@Beachman4
Copy link

Beachman4 commented Oct 24, 2024

When using this project in a react native/expo project, I can't seem to get it to play at all with the subpath import aliases. I suspect the issue lies with either babel or metro-bundler as I don't have this issue when running this in a standard node environment using typescript & tsc.

If I could make a suggestion and perhaps I'd be willing to implement this, is to utilize something like esbuild-plugin-alias-path for the js files? I don't think this issue affects the type files.

[Expo/React Native]
Node Version: 22.9.0
Npm: 10.8.3
@babel/core: ^7.25.0
react-native: 0.74.5
expo: ~51.0.28

(In Node env)
pnpm: 9.10.0

@Beachman4
Copy link
Author

Beachman4 commented Oct 24, 2024

On second thought that package won't work as it's meant to be used if you're bundling, I believe.

@lauckhart
Copy link
Collaborator

Metro added support for package.json "imports" field way back in version 0.67. But I assume it does the rewrite during bundling... Is the problem that you're importing the files directly without bundling?

The import aliases are a bit new for us... If necessary we can rewrite them during build.

@Beachman4
Copy link
Author

Beachman4 commented Oct 24, 2024

So the error I'm getting is:
Unable to resolve "#protocol" from "node_modules\@project-chip\matter.js\dist\esm\export.js"

I've tried just about every which way I can to get it to work, I even wrote my own "post processing" step in matter-build to traverse all built files and replace the aliases with either the packages they referenced or the relative path. (This did solve that error, but now I'm getting a different error)

Just found this issue & PR about adding subpath import alias support

@lauckhart
Copy link
Collaborator

OK, sounds like we should rewrite the aliases in our transpiler output. I'll try to get to that in the next couple of days.

@Beachman4
Copy link
Author

OK, sounds like we should rewrite the aliases in our transpiler output. I'll try to get to that in the next couple of days.

Cool, that PR I linked doesn't add support for external packages and I mentioned that, but looks like they'll do a second PR to address it. I finally got the imports working in my local, I just had to modify the metro resolver package to address some things I would consider are bugs, but then also add this to my local metro.config.js:

config.resolver.resolveRequest = (context, moduleName, platform) => {
    if (moduleName === 'crypto') {
        // when importing crypto, resolve to react-native-quick-crypto
        return context.resolveRequest(
            context,
            'react-native-quick-crypto',
            platform,
        )
    }
    if (moduleName.startsWith('#')) {
        const pkg = context.getPackageForModule(context.originModulePath);
        const importsField = pkg?.packageJson.imports;

        const importForField = Object.keys(importsField).find((item) => {
            let fix = item

            if (item.endsWith('/*')) {
                fix = fix.substring(0, fix.length - 2)
            }

            return moduleName.startsWith(fix)
        })

        if (importForField && importsField[importForField].startsWith('@matter')) {
            let foundValue = importsField[importForField]
            let foundKey = importForField
            if (foundKey.endsWith('/*')) {
                foundKey = foundKey.substring(0, foundKey.length - 2)
            }

            if (foundValue.endsWith('/*')) {
                foundValue = foundValue.substring(0, foundValue.length - 2)
            }
            const newModuleName =  moduleName.replace(foundKey, foundValue)

            const resolveVal = resolve(context,newModuleName, platform)

            const origin = context.originModulePath.split(path.sep).slice(-3).join(path.sep)

            return {
                ...resolveVal,
                filePath: resolveVal.filePath.replace(origin, '')
            }
        }
    }

    return context.resolveRequest(context, moduleName, platform)
}

I'm also encountering some other errors, like the hermes JS engine not liking the local eval used in GeneratedClass but I'll make separate issues for that

@Apollon77
Copy link
Collaborator

@Beachman4 One note for react native in general: It is not yet ready. We lost the interested parties somewhere during the way to commission a device. We already fixed a lot of things but I suspect that there is still some stuff in crypto missing because Matter has really high requirements and I think AES-CCM should still be an issue. So you might run against this when you do a controller or device and try using it.

Because @lauckhart and me are not focussing on mobile currently we would be happy about all support and are also willing to try to help the best we can. If you like or want more direct communication just join the Discord.

@Beachman4
Copy link
Author

@Beachman4 One note for react native in general: It is not yet ready. We lost the interested parties somewhere during the way to commission a device. We already fixed a lot of things but I suspect that there is still some stuff in crypto missing because Matter has really high requirements and I think AES-CCM should still be an issue. So you might run against this when you do a controller or device and try using it.

Because @lauckhart and me are not focussing on mobile currently we would be happy about all support and are also willing to try to help the best we can. If you like or want more direct communication just join the Discord.

Ahh I see, well I'm happy to work with it and potentially contribute to getting it to a working solution. I'll join the discord shortly.

My ultimate goal is to get PASECommissioning working, use the network commissioning cluster to scan and then set the wifi or thread network and then hand off to another device on network to do the full commissioning into the fabric

@Apollon77
Copy link
Collaborator

matter.js in general is prepared for such a "split" process.

Please not one thing: Apple requires to use their Matter services for initial commissioning ... so formally you are not allowed to do this yourself. Google might do the same somewhen, but could allow it today.
So basically thats the reason other devs stopped .... :-(

@Beachman4
Copy link
Author

matter.js in general is prepared for such a "split" process.

Please not one thing: Apple requires to use their Matter services for initial commissioning ... so formally you are not allowed to do this yourself. Google might do the same somewhen, but could allow it today. So basically thats the reason other devs stopped .... :-(

Yeah you are correct, it appears if you wanted your app published on the app store, you'd need to write some native code and use their framework, which is a bummer, but it's hardly surprising given that it's Apple lol. Of course this is about publishing so if you'd just want to tinker then you'd be fine.

I can't find anything saying google play store has the same requirement so it should be fine in theory.

I did have to make some changes in order to get the sdk working on Hermes(jsc was just not working), mainly around GeneratedClass. Hermes does not support local eval, which currently GeneratedClass uses to dynamically create a new class object, so I had to change it to use raw javascript, prototypes and whatnot to create the class

@lauckhart
Copy link
Collaborator

I did have to make some changes in order to get the sdk working on Hermes(jsc was just not working), mainly around GeneratedClass. Hermes does not support local eval, which currently GeneratedClass uses to dynamically create a new class object, so I had to change it to use raw javascript, prototypes and whatnot to create the class

Originally we used es6 class syntax but without the eval for class generation. Only reason for the eval is so every instance is called "klass" or something in the debugger. :) Might make sense to have a static flag on GenerateClass that enables your behavior (generating the class without the eval), would you be able to do something like that with your code?

@lauckhart
Copy link
Collaborator

And back to the original topic here... Esbuild doesn't run resolve plugins unless bundling is enabled... Replacing the imports after transpile wouldn't be too bad but we'd have to update the sourcemaps too which would probably be both slow and a PITA.

Option 2 would be for us to stop using the import aliases, but they seem pretty well supported and save us from a lot of ../../..'s in our code.

Option 2 wouldn't be the end of the world but WDYT about using your current solution, bundling, and/or continue working w/ the metro guys?

@Beachman4
Copy link
Author

And back to the original topic here... Esbuild doesn't run resolve plugins unless bundling is enabled... Replacing the imports after transpile wouldn't be too bad but we'd have to update the sourcemaps too which would probably be both slow and a PITA.

Option 2 would be for us to stop using the import aliases, but they seem pretty well supported and save us from a lot of ../../..'s in our code.

Option 2 wouldn't be the end of the world but WDYT about using your current solution, bundling, and/or continue working w/ the metro guys?

I mean the import aliases are a native node feature, so it's up to the metro guys to add support for it, which they are, so I don't think anything needs to change right now for this project. I gave them some feedback on that PR and asked for support for aliasing to external packages(as the PR only supports in-package aliases), which they mentioned they would do it in a follow up PR.

My current solution is using the code from that WIP PR and then a custom resolve request function in the metro config, so this is no longer a blocker for me.

@Beachman4
Copy link
Author

I did have to make some changes in order to get the sdk working on Hermes(jsc was just not working), mainly around GeneratedClass. Hermes does not support local eval, which currently GeneratedClass uses to dynamically create a new class object, so I had to change it to use raw javascript, prototypes and whatnot to create the class

Originally we used es6 class syntax but without the eval for class generation. Only reason for the eval is so every instance is called "klass" or something in the debugger. :) Might make sense to have a static flag on GenerateClass that enables your behavior (generating the class without the eval), would you be able to do something like that with your code?

Yeah I think that's a good idea, I'm not too sure that my solution works 100%, I mean all the tests pass, but that functionality is used by so many generated files, I'm unsure if it works 100%. I'll throw up a PR to add that static flag and toggle it from the react native platform file in @matter/main

@lauckhart
Copy link
Collaborator

Great, thanks. if the tests pass it's probably good, we have a lot of coverage there

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

No branches or pull requests

3 participants